Simple calculator in C V2

Question

My last question had a lot of views (over a 1000) so I decided to apply what I learned from the previous question and revamp the program. I learnt a lot from posting that question (functions, how to do a menu without using using a lot of printf and learning to not repeat code etc). So here's the updated program:

#include <stdio.h>
#include <stdlib.h>
//Function prototype
void getData(int *,int *);
int addition(int,int);
int subtraction(int,int);
int multiplication(int,int);
int division(int,int);
//Main function
int main()
{
 do//Infinite do - While loop
 {
 int num1,num2,choice;
 printf("\nEnter a number from the list below\n\n"
 "\t1. Addition\n"
 "\t2. Subtraction\n"
 "\t3. Multiplication\n"
 "\t4. Division\n"
 "\t0. Quit program\n");
 printf("\nEnter number: ");
 scanf("%d",&choice);
 if (choice == 0)
 {
 puts("\nExiting program...");
 exit(0);
 }
 else
 {
 getData(&num1,&num2);//Get values for user input
 if(choice == 1)
 {
 printf("\n%d + %d = %d\n",num1,num2,addition(num1,num2));//Output addition
 }
 else if(choice == 2)
 {
 printf("\n%d - %d = %d\n",num1,num2,subtraction(num1,num2));//Output subtraction
 }
 else if(choice == 3)
 {
 printf("\n%d * %d = %d\n",num1,num2,multiplication(num1,num2));//Output multiplication
 }
 else if(choice == 4)
 {
 if(num2 == 0)
 {
 printf("\nError! Division by zero! Exiting program\n");
 exit(1);
 }
 else
 {
 printf("\n%d / %d = %d\n",num1,num2,division(num1,num2));//Output division
 }
 }
 else
 {
 printf("Enter correct number! Exiting program");
 exit(1);
 }
 }
 }while(1);
}
//Get memory address of num1, num2 and store values in this location
void getData(int *num1_ptr,int *num2_ptr)
{
 printf("\nEnter first number: ");
 scanf("%d",num1_ptr);
 printf("\nEnter second number: ");
 scanf("%d",num2_ptr);
}
//Addition
int addition(num1,num2)
{
 return num1 + num2;
}
//Subtraction
int subtraction(int num1,int num2)
{
 return num1 - num2;
}
//Multiplication
int multiplication(int num1,int num2)
{
 return num1 * num2;
}
//Division
int division(int num1,int num2)
{
 return num1 / num2;
}
//End of code

• Spikatrix: Check the return value of scanf. Otherwise, your code will exhibit Undefined Behavior if the user enters invalid data such as characters.

Answer 1

Other reviews already provided some pretty good points, so I'll just mention a few other things and reiterate on some already mentioned ones.

• I would advise against function prototypes in this case. Since this is a single source file program, you could declare all functions before main() and avoid the prototypes. The problem with function prototypes is code duplication. If you change one function, at least two updates are mandatory, instead of just one. Avoid this extra maintenance overhead if you can.

• For decimal division to work properly, you'll need a to use a floating-point type. My suggestion would be to use double instead of float. The float type is only recommended for storage and memory saving. Most CPUs operate natively on doubles and will convert a float to double when operating on a value. Since you don't have storage constraints, double is the best option.

• Don't write noisy and obvious comments. The overall rule about code commenting is to use them to explain why you solved a problem in a given way, or to give any extra information (web links, mathematical formulas, etc) that will aid the understanding of a piece of code. Explaining what a code is doing should not be necessary if the code is well written. Well written code is self explanatory.

• Arguably personal preference, but I find it better to properly space expressions and parameter list in function calls.

  printf("\n%d + %d = %d\n",num1,num2,addition(num1,num2));
  

  Better spacing:

  printf("\n%d + %d = %d\n", num1, num2, addition(num1, num2));
  

Answer 2

I wonder why you switched from floats to ints. This causes two problems with division:

• Division is not closed over integers. That is, the result of dividing two integers is not necessarily an integer. So, you are truncating the result without warning.
• Integer division by 0 is an error; floating-point division by 0.0 works. Now you have the complication of having to handle that error. I would expect that the program should continue after printing the error message, rather than exiting.

Answer 3

The program looks so much better. Few more tips:

• When someone divides by 0, instead of exiting the program, prompt them to re-enter the second number

• I noticed you changed num1 num2 to int. If someone inputs a number with decimals, the program misbehaves. It's probably best to leave the variables as float.

• You now also have a nested if-else loop instead of switch (case). I don't know how I feel about that. It probably doesn't matter, and people may disagree, but I'd go back to switch (case). It just looks neater and I find it to be the go-to way to work menus.

• If someone enters wrong values for choice, the program still prompts the user for num1 and num2, then prints Enter correct number! Exiting program. If you had a switch (case) menu, you could add a default case that would automatically tell the user that the choice is incorrect and they need to pick a valid option

• You should stick with int main(void) in C. See: https://stackoverflow.com/a/12225214/4907651

• You should keep return 0 for good practice

• Too much (useless) commenting. You don't need to add a comment stating when the main starts or what the function does unless it's not obvious.

I do like how when you exit the program, you have exit(0) for when it exits by user command (successful execution), and exit(1) for when there is an error.

• Spikatrix: return 0 isn't needed. It is implicitly supplied in C99 and above, IIRC.
• 200_success: Item 4 has nothing to do with if-else vs. switch. It's just that the exit(1) is probably a bad idea.
• Ishaan: @CoolGuy I only knew about the implicit addition of return 0 in c++. Thanks for pointing that out.
• Daniel Morris: Completely forgot about making them floats, was looking at another program that had the infinite loop and must have forgot about changing it. I'll update that right away.
• Daniel Morris: How would you go about stopping the user from entering a decimal in this section: printf("\nEnter number: "); scanf("%d",&choice);

Answer 4

Some additional remarks on top of all the other responses:

Avoid do {} while (1); for endless loops. The classic idiom is for (;;) { }. The rationale is that for (;;) is more concise and provides the endless loop info immediately, whereas do {} while (1); keeps this info from the reader until the very end of the statement, which is more than one page long in this example.

getData() may fail but scanf() failure is not tested and cannot be returned to the caller, invoking undefined behaviour.

num2 == 0 is not the only case of integer division overflow. INT_MIN / -1 will overflow and invoke undefined behaviour, typically ending the program with an uncaught signal.