should this work

Discussion in 'C Programming' started by amanayin, Oct 19, 2003.

  1. amanayin

    amanayin Guest

    The program is simple and i know i should use switch instead of if
    but i done it this way for a reason. But i am not sure if it should work
    when i was compiling it i was getting warning messages but then suddenly
    it started working. The reason i wrote the program was to get used to using
    functions, if statement and logical operators.

    /* ARITH1.C SIMPLE CALCULATOR PROGRAM */

    #include<stdio.h>

    float add(float a, float b);
    float sub(float a, float b);
    float multi(float a, float b);
    float div(float a, float b);
    float per(float a, float b);
    char call(float math);
    float a,b,c,math;
    char d;

    int main(void)
    {
    printf("Enter 1st value: ");
    scanf("%f",&a);
    printf("Enter 2nd value: ");
    scanf("%f",&b);

    call(d);
    return 0;
    }


    char call(float math)
    {

    while(d != 'q'){
    printf("Enter operand or q to exit: ");
    scanf("%s",&d);

    if(d == '+')
    { math = add(a,b); }
    if(d == '-')
    { math = sub(a,b); }
    if(d == '*')
    { math = multi(a,b); }
    if(d == '/')
    { math = div(a,b); }
    if(d == '%')
    { math = per(a,b); }
    if(d != '+' && d != '-' && d != '*' && d != '/' && d != '%' && d != 'q')
    { printf("Operand not accepted try again\n"); continue; }
    if(d == 'q')
    { puts("Exiting"); break; }
    printf("Total is %.3f\n",math);
    }
    return math;
    }

    float add(float a, float b)
    {
    c = a + b;
    return c;
    }

    float sub(float a,float b)
    {
    c = a - b;
    return c;
    }

    float multi(float a, float b)
    {
    c = a * b;
    return c;
    }

    float div(float a, float b)
    {
    c = a / b;
    return c;
    }

    float per(float a, float b)
    {
    c = a * b / 100;
    return c;
    }
     
    amanayin, Oct 19, 2003
    #1
    1. Advertisements

  2. amanayin

    Mark Gordon Guest

    Why declare these as globals? It is far easier to get things right if
    you minimise the scope of variables.
    call is declared as taking a float but you are passing a character. You
    compiler should have warned you about this. Also d has not been
    initialised and by passing it as a parameter you are evaluating it.
    You never use the value passed in so why have it as a parameter?
    d still has not been initialised so absolutely ANYTHING could happen
    here.
    This may not actually be printed before the scanf. Read the FAQ for
    further information.
    %s fetches a null terminated string, not a single character but you are
    only passing the address of a single character, so this is guaranteed to
    overflow on any input.
    The break will exit the while loop making the condition completely
    pointless once all the other bugs are fixed.
    Why are you using a global variable here? Why not just use
    return a + b;
    <snip>

    I seriously think you need to do work through a decent text book. You
    will find recommendations in the FAQ for comp.lang.c (Q 18.10)
     
    Mark Gordon, Oct 19, 2003
    #2
    1. Advertisements

  3. 'div' is a bad name for a function, because there's a function with the
    same name declared in stdlib.h. Not a problem yet, but if you have to
    include stdlib.h later the two definitions will clash. Call it 'divide'
    or the like.
    You pass a character to a funtion that expects a double.
    1. Why do you pass an argument at all?
    2. Your function is declared to return a character.
    Later on you return a double.
    The "%s" conversion specifier is for reading strings. It will store any
    user input plus a null character at &d, but there's only space for one
    character. Use "%c" (overkill) or just d = getchar(); (recommended).
    See above.
    See above.
    Besides of the mentioned problems your code looks fine AFAICT.

    HTH

    Regards
     
    Irrwahn Grausewitz, Oct 19, 2003
    #3
  4. amanayin

    osmium Guest

    I wouldn't do that. You are passing call() a char when it wants a float.
    There is not reson to do that in code such as this. It may have no bearing
    on the problem you say you *used to have*.
     
    osmium, Oct 19, 2003
    #4
  5. s/double/float/
     
    Irrwahn Grausewitz, Oct 19, 2003
    #5
  6. d has been initialized to 0 automatically on declaration.

    Regards
     
    Irrwahn Grausewitz, Oct 19, 2003
    #6
  7. amanayin

    Mark Gordon Guest

    You're right. I saw so many problems in how the code was written I saw
    more problems than were there.
     
    Mark Gordon, Oct 19, 2003
    #7
  8. amanayin

    pete Guest

    I think it is a problem.

    My copy of the C89 last draft, has this:
    4.1.2 Standard headers
    All external identifiers declared in any of the headers are
    reserved, whether or not the associated header is included.
     
    pete, Oct 20, 2003
    #8
  9. Why would you expect a warning when the conversion from char to float
    is automatic and well defined?



    <<Remove the del for email>>
     
    Barry Schwarz, Oct 20, 2003
    #9
  10. You're right. I should have said: "Possibly not causing real trouble
    yet, but definitely a problem ..."

    Regards
     
    Irrwahn Grausewitz, Oct 20, 2003
    #10
  11. amanayin

    amanayin Guest

    Irrwahn Grausewitz wrote:

    snip
    If i use d = getchar(); or %c instead of %s i get the
    following out put
    Enter operand or q to quit: operand not accepted try again:
    change it back to %s it works fine
     
    amanayin, Oct 20, 2003
    #11
  12. amanayin

    amanayin Guest

    Mark Gordon wrote:

    so what should i change it to?
     
    amanayin, Oct 20, 2003
    #12
  13. amanayin

    Mark Gordon Guest

    When you want a variable within a function you should declare it within
    that function.

    From memory something like

    void call(void)
    {
    float math;
    /* do stuff */
    }

    would have been better.
     
    Mark Gordon, Oct 20, 2003
    #13
  14. Er, yes, that's because scanf left some unread input (the newline, at
    least) after the second value was read. In the code below this is
    fixed (see function drain_stdin()).
    And this still invokes undefined behaviour. d can only hold one
    character, thus you produce a buffer overrun.

    In the code below the global variables are eliminated and the definition
    of call is fixed. I left the if-chain unchanged, as you said it was an
    exercise. Note also the use of fflush(), lookup the c.l.c-FAQ Q12.4 for
    an explanation. You should also consider to use double instead of
    float; usually there's no reason to use float at all.

    Here's the improved code:

    /* ARITH2.C SIMPLE CALCULATOR PROGRAM */

    #include<stdio.h>

    float add(float a, float b);
    float sub(float a, float b);
    float multi(float a, float b);
    float div(float a, float b);
    float per(float a, float b);
    void call( float a, float b );
    void drain_stdin( void );

    int main(void)
    {
    float a, b;

    printf("Enter 1st value: ");
    fflush( stdout );
    scanf("%f",&a);
    printf("Enter 2nd value: ");
    fflush( stdout );
    scanf("%f",&b);

    call( a, b );
    return 0;
    }


    void call( float a, float b )
    {
    char d = 0;
    float math = 0.0;

    while( d != 'q' )
    {
    printf("Enter operand or q to exit: ");
    fflush( stdout );
    drain_stdin();
    scanf( "%c", &d );

    if(d == '+')
    { math = add(a,b); }
    if(d == '-')
    { math = sub(a,b); }
    if(d == '*')
    { math = multi(a,b); }
    if(d == '/')
    { math = div(a,b); }
    if(d == '%')
    { math = per(a,b); }
    if(d != '+' && d != '-' && d != '*' && d != '/' &&
    d != '%' && d != 'q')
    { printf("Operand not accepted try again\n"); continue; }
    if(d == 'q')
    { puts("Exiting"); break; }
    printf("Total is %.3f\n", math);
    }
    return;
    }

    void drain_stdin( void )
    {
    while ( getchar() != '\n' )
    /*do nothing*/;
    return;
    }

    float add(float a, float b)
    {
    return a + b;
    }

    float sub(float a,float b)
    {
    return a - b;
    }

    float multi(float a, float b)
    {
    return a * b;
    }

    float div(float a, float b)
    {
    return a / b;
    }

    float per(float a, float b)
    {
    return a * b / 100;
    }


    HTH
    Regards
     
    Irrwahn Grausewitz, Oct 21, 2003
    #14
  15. amanayin

    amanayin Guest

    Thanks for that
     
    amanayin, Oct 21, 2003
    #15
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.