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

1. ## simple calculator: what's wrong with this code?

Pls help
Thank you

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

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

int main()
{
int num1;
int num2;
int rsult = 0;
char oprtn;
printf("Enter the first number: ");
scanf("%i", &num1);
printf("Enter the second number: ");
scanf("%i", &num2);
printf("Choose an operator (a/s/m/d): ");
scanf("%s", &oprtn);
switch(oprtn)
{
case 'a':
rsult = num1 + num2;
break;

case 's':
rsult = num1 - num2;
break;

case 'm':
rsult = num1 * num2;
break;

case 'd':
rsult = num1 / num2;
break;
}

getch();
}```

test:
num1 = 5
num2 = 6
oprtn = a
gives a result of 5

2. 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.

3. There are a few things wrong, but it's helpful to include what the output is, as well as the input (In this case input is pretty obvious).

You are using a char to store a string format, it's probably easier to use a %c format. Also, you will need to use float's for division (it commonly produces decimal leftover). The getch() function is from conio.h, so you will need to include it (getchar() would probably be more portable though).

4. OK forget using char.
Even if I use int, it ALWAYS gives me the the value of x as the output
Code:
```/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */

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

int main()
{
int x;
int y;
int rsult = 0;
char oprtn;
printf("Enter the first number: ");
scanf("%i", &x);
printf("Enter the second number: ");
scanf("%i", &y);
printf("Choose an operator\n");
scanf("%i", &oprtn);
switch(oprtn)
{
case 1:
rsult = x+y;
break;

case 2:
rsult = x-y;
break;

case 3:
rsult = x*y;
break;

case 4:
rsult = x/y;
break;
}

return 0;
}```

5. Don't go in too deep with %i, just use %d.

6. Even if I use int, it ALWAYS gives me the the value of x as the output
Are you getting warnings when you compile?

Code:
```/*
||=== scrap_c, Debug ===|
main.c||In function 'main':|
main.c|18|warning: format '%i' expects type 'int *', but argument 2 has type 'char *'|
||=== Build finished: 0 errors, 1 warnings ===|
*/```
Also, why are you trimming your variable names? "result" is much easier to read/write/remember than "rsult" - ditto for "operation" versus "oprtn".

7. Edit:
Code:
```/* A simple program in which the user picks 2 numbers and does mathematical operation ( +, -, * or / )  */

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

int main()
{
int x;
int y;
int result = 0;
char operation;
printf("Enter the first number: ");
scanf("%i", &x);
printf("Enter the second number: ");
scanf("%i", &y);
printf("Choose an operator\n");
scanf("%i", &operation);
switch(operation)
{
case 1:
result = x+y;
break;

case 2:
result = x-y;
break;

case 3:
result = x*y;
break;

case 4:
result = x/y;
break;
}
getch();

return 0;
}```

No errors, no warnings

8. Originally Posted by Owais
Don't go in too deep with %i, just use %d.
Tried. Still gives me the output of x instead of result

9. 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':
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]

10. 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;
break;

case 3:
result = x*y;
break;

case 4:
a = (float) x;     //recasting results to float for division
b = (float) y;     //recasting results to float for division
divresult = a/b;
break;

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

11. 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;
break;

case 3:
result = x*y;
break;

case 4:
a = (float) x;     //recasting input to float for division
b = (float) y;     //recasting input to float for division
divresult = a/b;
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;
}```

12. 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.

13. 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.

14. Originally Posted by spongefreddie
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;
break;

case 3:
result = x*y;
break;

case 4:
a = (float) x;     //recasting results to float for division
b = (float) y;     //recasting results to float for division
divresult = a/b;
break;

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

omg thank you so much!

15. 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.