Fred
9/16/2011 6:38:00 PM
On Sep 15, 2:52 pm, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> Andy Foot <f...@dev.null.invalid> writes:
> > I have recently started a distance-learning C course.
>
> I won't re-answer your question, but there are a number of things about
> your code that make me a little worried about this course. Of course,
> it may be that you've gone on ahead and some of these issues come from
> an admirable sense of exploration. In which case, just make sure that
> when you explore you do so with some good documentation.
>
> > #include <stdio.h>
> > void main()
>
> main returns int. It's also a good habit to write (void) rather than ()
> when a function takes no arguments. It make no difference here, but
> there is no reason to do it right from the very start. The reason it
> matters in bigger programs is that the f() form is old syntax that has a
> special, rather lax, meaning. The f(void) form instructs the compiler
> to tell you when you call f with arguments and you always want as much
> help as possible from the compiler.
>
> > {
> > char buf[BUFSIZ];
>
> There's no particular reason to use BUFSIZ here. Someone who does not
> know what it is will wonder if it's big enough (or too big) and someone
> who does know will wonder why that size is important to your program.
> I'd pick an explicit size myself.
>
> > int a=0,b=0;
>
> Those are not really very helpful names. I know it seems daft, in a
> half dozen line program, to worry about names, but why not start out by
> thinking up good names? They don't have to be long: "count" and "sum"
> would be OK, I think.
>
> > while(strcmp(gets(buf),"")!=0){
>
> There are several issues here. One is that gets is a really bad
> function -- so bad it has suffered the ultimate fate of being removed
> form the next C standard and it is officially deprecated in the current
> one. The second issue is that gets returns a null pointer when the end
> of the input is reached. Passing a null pointer as one of strcmp's
> arguments is a bad thing -- anything can happen.
>
> The possibility of the input running out may not be something that you
> care about right now since you are typing at your program but, again,
> it's good to avoid getting into bad habits. Whilst I'd not use gets,
> you could fix this particular issue like this:
>
> while (gets(buf) != NULL && strcmp(buf, "") != 0) { ...
>
> The third issue is just whether you are testing the right thing. Your
> test stop the loop when an empty line is typed but does not stop when
> there is no sane input (for example "xyz" is not a number but you go
> ahead and try to use it as one).
>
> I'd use scanf for this. scanf has a bad reputation amongst C experts
> but it is actually very good for this sort of program. It's bad rap
> comes from the fact that you can't easily control the input format, but
> that is not a worry to you here. It has the lovely property of telling
> you when things have worked out -- if you scan for one int, it returns 1
> when an it was found so can simply write:
>
> while (scanf("%d", &b) == 1) {...
>
> Any errors and the loop stops. Of course that may not be what was
> wanted and you may not have been told about scanf yet (and it's rather
> odd &b argument) but using gets and atoi does not seem like a good thing
> to learn to do.
>
> If scanf is not an option, use fgets instead of gets. It gets told how
> big the buffer is so it can't go wrong in the same horrible ways that
> gets can.
>
> > a++; b+=atoi((char*)buf);
>
> The cast is not needed. It has no effect.
>
> > }
> > printf("mean=%f",(float)(b/a));
>
> You should have a \n after the %f. Again it's a matter of good habits.
> Your system may well be happy with what you have but failing to end your
> output with a newline can mess up on some systems.
>
> > }
>
> It's better to end main with "return 0;". The latest C standard has a
> special exemption for the main function but, again, it's a good habit to
> get into -- if a function has a value, you must remember to return one.
> I know that in this case, you didn't because you though main was a void
> function so this really the first just my point all over again.
>
> This is, I know, a lot of comments on a small program. Please don't be
> discouraged. Think of it like a brain dump -- not all of them are
> important but I find it easier just to say everything that comes into my
> head.
>
This code can still fail.
Try to fond the mean of these three numbers:
MAX_INT - 1
MAX_INT - 2
MAX_INT - 3
The mean is (MAX_INT-2), but the algorithm used will break due to
overflow.
--
Fred K