Thread: simple calculator: what's wrong with this code?

This is NOT defined; it could do anything. (When oprtn is of char type)

Code:

scanf("%s", &oprtn);

I suggest trying; note the space before the % is needed to skip newline/space.

Code:

scanf(" %c", &oprtn);

Tim S.

Compile at maximum warning level, you should see something like the following:

Code:

$ make foo
gcc -Wall -ggdb3 -pedantic -std=gnu99 -O0 -o foo foo.c -lm -lpthread -lrt
foo.c: In function ‘main’:
foo.c:16:5: warning: format ‘%i’ expects argument of type ‘int *’, but argument 2 has type ‘char *’ [-Wformat]
foo.c:36:5: warning: implicit declaration of function ‘getch’ [-Wimplicit-function-declaration]
/tmp/ccMa7s5g.o: In function `main':
/usr/local/google/home/cagarvin/sandbox/cprogramming/foo.c:36: undefined reference to `getch'
collect2: ld returned 1 exit status

You must use the correct scanf format specifier for the type you are reading into. As you have been told before, use " %c" (note the space) for reading in operation.\

Second, you need to realize that '1' and 1 are different. '1' is a character, for example, read by %c with scanf. 1 is a number, read with %d or %i for scanf. Decide whether you want operation to be a char or an int. Then, make sure you declare it of the right type, use the right format specifier for scanf, and use the correct value in your case statement:

Code:

case '1': // if operation is a char -- note the ' ' single quotes
// or
case 1:  // if operation is an int

[EDIT]
Ultimately, you should learn to use a debugger so you can trace code line by line and examine all relevant values. Pending that, at least get in the habit of using printf statements to print important values at critical points in your program. If you printed operation with "%i", you would likely see it has a value like 48, instead of the expected 1, which tells you you're reading it wrong instead of it being calculated incorrectly. In fact, it was never calculated since you didn't hit any of the switch statements -- like all uninitialized automatic variables, it has an arbitrary garbage value.

You should get in the habit of always putting a default: case in your switch statements to catch cases like this. Have it print/log an error message to help you debug.

Also, I should point out that on my system, it gave 0 on one run, even though x was 5. Making incorrect/overly specific assumptions like "it gives the value of x" can make it more difficult to determine what is actually happening. You get stuck trying to figure out how it gets the value of x. Start with a less specific assumption like "result has the incorrect value". Then, work on determining where that incorrect value came from: unassigned variable, copied value of x, garbage value from elsewhere (e.g. buffer overflow), etc.
[/EDIT]

Originally Posted by zer0_r00t

Edit:

Code:

/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */


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

<SNIP>

No errors, no warnings

Here's how I made it work in code::blocks:

Code:

/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */

#include <stdio.h>      //this is all you need for your code, getchar is included

int main(void)
{
    int x, y;
    float a, b;
    int result = 0;
    float divresult = 0.00;                  //just a habit of mine
    int operation;
    printf("Enter the first number: ");
    scanf("%d", &x);                         //using recommended %d instead of %i
    printf("Enter the second number: ");
    scanf("%d", &y);
    printf("Choose an operator\n");
    printf("1 = add, 2 = subtract, 3 = multiply, 4 = divide\n");  //more attractive to user
    scanf("%d", &operation);                                      //float is only needed for result, not integer input
    switch(operation)
    {
    case 1:
        result = x+y;
        printf("Your result is %d", result);    //seemed simpler to include result line
        break;                                  //in every case instead of trying to
                                                //trying to sort it out outside of switch
    case 2:
        result = x-y;
        printf("Your result is %d", result);
        break;

    case 3:
        result = x*y;
        printf("Your result is %d", result);
        break;

    case 4:
        a = (float) x;     //recasting results to float for division
        b = (float) y;     //recasting results to float for division
        divresult = a/b;
        printf("Your result is %f", divresult);
        break;

    default:
        printf("You made a boo-boo. Run program again.");
    }
    getchar();              //I assume you wished to pause console program before exit
    return 0;
}

Originally Posted by zer0_r00t

Edit:

Code:

/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */


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

<SNIP>

No errors, no warnings

Here's how I made it work in code::blocks:

Code:

/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */

#include <stdio.h>      //this is all you need for your code, getchar is included

int main(void)
{
    int x, y;
    float a, b;
    int result = 0;
    float divresult = 0.00;                  //0.00 just a habit of mine
    int operation;
    printf("Enter the first number: ");
    scanf("%d", &x);                         //using recommended %d instead of %i
    printf("Enter the second number: ");
    scanf("%d", &y);
    printf("Choose an operator\n");
    printf("1 = add, 2 = subtract, 3 = multiply, 4 = divide\n");  //more attractive to user
    scanf("%d", &operation);                                      //float is only needed for result, not integer input
    switch(operation)
    {
    case 1:
        result = x+y;
        printf("Your result is %d", result);    //seemed simpler to include result line
        break;                                  //in every case instead of trying to
                                                //manage it outside of switch
    case 2:
        result = x-y;
        printf("Your result is %d", result);
        break;

    case 3:
        result = x*y;
        printf("Your result is %d", result);
        break;

    case 4:
        a = (float) x;     //recasting input to float for division
        b = (float) y;     //recasting input to float for division
        divresult = a/b;
        printf("Your result is %f", divresult);
        break;

    default:
        printf("You made a boo-boo. Run program again.");    //why not?
    }
    getchar();              //I assume you wished to pause console program before exit
    return 0;
}

Sorry about the dual replies; I attempted to edit my post and the server stopped responding. When the server's flatulence was finished, I was rewarded with two permanent posts, and an inabilty to delete #10. The only difference between the two is in the edited comments in the code, not the code itself.

Cheers.

spongefreddie: Do not hand out complete solutions! It is against the forum rules, and is just generally considered bad form, because the OP learns nothing from it.

Originally Posted by Elkvis

spongefreddie: Do not hand out complete solutions! It is against the forum rules, and is just generally considered bad form, because the OP learns nothing from it.

I apologize. Believe it or not, I was ignorant of that rule. It won't happen again.

I mistakenly thought that the comments I included would be instructive, as they explained the reasons for my alterations to produce a working program.

I am moved to comment that when I was first learning C, the cprogramming.com forum was a invaluable resource, because I always received answers to my questions. As I recall, no one ever provided the source for an entire program, but then again, the parts they did provide were the only parts I was coding incorrectly.

I will endeavor to be more instructive and less demonstrative in the future, if I post another response.