Arithmetic error

Discussion in 'C Programming' started by Andy Foot, Sep 15, 2011.

  1. Andy Foot

    Andy Foot Guest

    Hello

    I have recently started a distance-learning C course.

    I am writing a program to input some numbers and calculate they're mean
    (average). However there is a bug: if the numbers are 1,2,3 then the
    calculation is correct (mean=2) however if the numbers are 1,2 then the
    answer is wrong (mean=1, should be 1.5).

    What's going on!

    Thanks,
    Andy

    #include <stdio.h>
    void main()
    {
    char buf[BUFSIZ];
    int a=0,b=0;
    while(strcmp(gets(buf),"")!=0){
    a++; b+=atoi((char*)buf);
    }
    printf("mean=%f",(float)(b/a));
    }
    Andy Foot, Sep 15, 2011
    #1
    1. Advertising

  2. Andy Foot

    Ian Collins Guest

    On 09/16/11 08:50 AM, Andy Foot wrote:
    > Hello
    >
    > I have recently started a distance-learning C course.
    >
    > I am writing a program to input some numbers and calculate they're mean
    > (average). However there is a bug: if the numbers are 1,2,3 then the
    > calculation is correct (mean=2) however if the numbers are 1,2 then the
    > answer is wrong (mean=1, should be 1.5).
    >
    > What's going on!
    >
    > #include<stdio.h>
    > void main()
    > {
    > char buf[BUFSIZ];
    > int a=0,b=0;
    > while(strcmp(gets(buf),"")!=0){
    > a++; b+=atoi((char*)buf);


    atoi should be avoided (it can't indicate an error), have a look at
    strtol instead.

    > }
    > printf("mean=%f",(float)(b/a));


    You are casting the result of (int)3/(int)2, which is 1. Make b a
    double and call it something meaningful like sum.


    --
    Ian Collins
    Ian Collins, Sep 15, 2011
    #2
    1. Advertising

  3. On Sep 15, 9:50 pm, Andy Foot <> wrote:

    > I have recently started a distance-learning C course.
    >
    > I am writing a program to input some numbers and calculate [their] mean
    > (average). However there is a bug: if the numbers are 1,2,3 then the
    > calculation is correct (mean=2) however if the numbers are 1,2 then the
    > answer is wrong (mean=1, should be 1.5).
    >
    > What's going on!


    you're doing integer maths when you mean to do floating point maths

    6 / 3 equals 2
    but 3 / 2 equals 1

    > #include <stdio.h>
    > void main()


    the correct signature for main is
    int main (void)
    ( or int main (int argc, char *argv[]) )

    main always returns int

    > {
    >     char buf[BUFSIZ];
    >     int a=0,b=0;


    these aren't very clear names. How about "count" and "sum"?

    >     while(strcmp(gets(buf),"")!=0){


    gets() should never be used. What if the user types more than BUFSIZ
    characters?

    >         a++; b+=atoi((char*)buf);


    atoi() has poor error handling. Use one of the alternatives (strtoul
    etc.). What is the point of the cast to char*? In general avoid casts
    unless you *really* need them.

    >     }
    >     printf("mean=%f",(float)(b/a));


    and this is where things go wrong

    printf ("mean=%f\n", (double)b / a);

    You need the \n or the output may not appear on some systems.
    Prefer double to float. Most of the C library uses double so it is
    rare float buys you anything.
    It is better if you use more whitespace in your layout. It makes it
    easier to read.

    Oh and since main *always* returns an int you need
    return 0;

    > }



    Happy programming, Nick
    Nick Keighley, Sep 15, 2011
    #3
  4. Andy Foot

    BartC Guest

    "Andy Foot" <> wrote in message
    news:j4toe7$1ls$...
    > Hello
    >
    > I have recently started a distance-learning C course.
    >
    > I am writing a program to input some numbers and calculate they're mean
    > (average). However there is a bug: if the numbers are 1,2,3 then the
    > calculation is correct (mean=2) however if the numbers are 1,2 then the
    > answer is wrong (mean=1, should be 1.5).
    >
    > What's going on!


    Try this version. The main change is moving the (float):

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>

    int main (void)
    {
    char buf[BUFSIZ];
    int a=0,b=0;
    puts("Enter one number per line. Blank line to end:");
    while(strcmp(gets(buf),"")!=0){
    a++; b+=atoi((char*)buf);
    }
    if (a) printf("Mean=%f\n",((float)b/a));
    }

    BTW you should use a newer textbook. (And where is BUFSIZ declared? Seems to
    compile though..)

    --
    bartc
    BartC, Sep 15, 2011
    #4
  5. Andy Foot <> 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.

    --
    Ben.
    Ben Bacarisse, Sep 15, 2011
    #5
  6. Andy Foot

    Ike Naar Guest

    On 2011-09-15, BartC <> wrote:
    > (And where is BUFSIZ declared? Seems to compile though..)


    <stdio.h>:
    The macros are [...] BUFSIZ which expands to an integer constant
    expression that is the size of the buffer used by the setbuf
    function;
    Ike Naar, Sep 15, 2011
    #6
  7. Andy Foot

    Ike Naar Guest

    On 2011-09-15, BartC <> wrote:
    > (And where is BUFSIZ declared? Seems to compile though..)


    <stdio.h>:
    The macros are [...] BUFSIZ which expands to an integer constant
    expression that is the size of the buffer used by the setbuf
    function;
    Ike Naar, Sep 15, 2011
    #7
  8. Andy Foot

    Fred Guest

    On Sep 15, 2:52 pm, Ben Bacarisse <> wrote:
    > Andy Foot <> 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
    Fred, Sep 16, 2011
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. joshc
    Replies:
    5
    Views:
    540
    Keith Thompson
    Mar 31, 2005
  2. mmacrobert
    Replies:
    1
    Views:
    312
    Victor Bazarov
    Aug 4, 2005
  3. darrel
    Replies:
    4
    Views:
    786
    darrel
    Jul 19, 2007
  4. Bill Cunningham

    pointer arithmetic error?

    Bill Cunningham, Dec 4, 2009, in forum: C Programming
    Replies:
    17
    Views:
    737
    Nick Keighley
    Dec 6, 2009
  5. Ross Culver

    Arithmetic Overflow Error When Click 'Last' Page Button

    Ross Culver, Aug 23, 2007, in forum: ASP .Net Web Controls
    Replies:
    0
    Views:
    132
    Ross Culver
    Aug 23, 2007
Loading...

Share This Page