Simple calculator in C

Question

I made a very simple calculator in C that allows the user to perform calculations like addition, subtraction, multiplication and division. This is my first C program that I've made outside of my University course. It would be great to get some feedback on this program so that I can avoid bad practices early on in my coding.

//Calculator program
#include <stdio.h>
#include <stdlib.h>

int main()
{
    int menu; //Variable for the number the user inputs
    float num1, num2, result; //Float variables for the user input and output, used floats in case the user enters e.g. 14.7

    printf("Enter a number from the list below\n\n");

    printf("1. Addition\n"); //Calculator menu, user must enter a value from 1 - 4 for the program to work
    printf("2. Subtraction\n");
    printf("3. Multiplication\n");
    printf("4. Division\n\n");

    printf("Enter number: "); //User input for the calculator menu
    scanf("%d", &menu);

    printf("\n");

    switch(menu) //switch statement for menu
    {
    case 1:
        printf("You entered Addition\n\n");

        printf("Enter first number: "); //User input for first number
        scanf("%f", &num1);

        printf("Enter second number: "); //User input for second number
        scanf("%f", &num2);

        printf("\n");

        result = num1 + num2; //Addition calculation

        printf("%.2f + %.2f = %.2f\n", num1, num2, result); //Addition output
        break;
    case 2:
        printf("You entered Subtraction\n\n");

        printf("Enter first number: "); //User input for first number
        scanf("%f", &num1);

        printf("Enter second number: "); //User input for second number
        scanf("%f", &num2);

        printf("\n");

        result = num1 - num2; //Subtraction calculation

        printf("%.2f - %.2f = %.2f\n", num1, num2, result); //Subtraction output
        break;
    case 3:
        printf("You entered Multiplication\n\n");

        printf("Enter first number: "); //User input for first number
        scanf("%f", &num1);

        printf("Enter second number: "); //User input for second number
        scanf("%f", &num2);

        printf("\n");

        result = num1 * num2; //Multiplication calculation

        printf("%.2f * %.2f = %.2f\n", num1, num2, result); //Multiplication Output
        break;
    case 4:
        printf("You entered Division\n\n");

        printf("Enter first number: "); //User input for first number
        scanf("%f", &num1);

        printf("Enter second number: "); //User input for first number
        scanf("%f", &num2);

        printf("\n");

        result = num1 / num2; //Division calculation

        printf("%.2f / %.2f = %.2f\n", num1, num2, result);
        break;
    default:
        printf("Enter correct number e.g 1 - 4\n"); //Outputted if the user enters a value other than 1 - 4
        break;
    }
    return 0;
}
//End of code

Answer 1

I must say, I respectively disagree with @ishyfishy's example of the addition function. Although, I do agree with the point about splitting the arithmetic functions away from the main function.

I believe that the arithmetic functions should only handle performing the arithmetic; all the I/O should be handled by main function (I have been taught that this is a good practice, although I do not have a source right now).

So, the "1" (addition) section of the switch statement in main would look something like this:

scanf("%f", &num1);
scanf("%f", &num2);

result = addition(num1, num2);

printf("...", num1, num2, result);

And the addition function simply looks like this:

float addition(float num1, float num2) {
    return num1 + num2;
}

---

This doesn't make a much of a difference, but it speeds up your code a little bit.

Instead of storing the result of a calculation in result, it would be faster to just pass the return value an arithmetic functions into the statement/function that you will it next in.

For example, the end of the "1" (addition) section of the switch statement would look like this:

printf("%.2f + %.2f = %.2f\n", num1, num2, addition(num1, num2)); 

This is faster because it reduces memory interaction(however, not by much).

---

In this section:

printf("Enter a number from the list below\n\n");

printf("1. Addition\n"); //Calculator menu, user must enter a value from 1 - 4 for the program to work
printf("2. Subtraction\n");
printf("3. Multiplication\n");
printf("4. Division\n\n");

You make 5 calls to the printf function when you could easily just put all the strings into one and split them up by newline in a single printf call:

printf("Enter a number from the list below\n\n"
       "1. Addition\n"
       "2. Subtraction\n"
       "3. Multiplication\n"
       "4. Division\n");

This is also faster as you don't have to call a function 5 times.

---

Only use printf when you are need to use the formatting feature. Other than that, you are over-complicating things.

If you merely need to output a string with a newline here or there, use the built-in puts function; it automatically outputs a string with a newline on the end (and you can add more if needed).

For example, this line:

printf("You entered Addition\n\n");

Becomes:

puts("You entered Addition\n");

Answer 2

1. I would give the user a chance to correct typing errors instead of just dumping them out of the program. Something like this, maybe:

   bCont = false;      // flag to denote good input, will be true with a good input.
   
   printf("Enter a number from the list below\n\n");
   printf("1. Addition\n"); 
   printf("2. Subtraction\n");
   printf("3. Multiplication\n");
   printf("4. Division\n\n");
   
   do
   {
       printf("Enter number: "); //User input for the calculator menu
       scanf("%d", &menu);
   
       if((menu < 1) || (menu > 5))
       {
            printf("invalid entry, please try again.");
       }
       else
       {
             bCont = true;
       }
    } while(!bCont);
   

2. You should check for the possibility of the user entering zero for the denominator in the division case, and print a warning. Something like this:

   if(0 != num2)
   {
       result = num1 / num2; //Division calculation
   }
   else
   {
       printf("Attempting division by zero, exiting");
       exit(1)
   }
   

3. I dislike repeated code, and you have a lot of it. I would rewrite the switch statement something like what is shown below.

   printf("Enter first number: "); //User input for first number
   scanf("%f", &num1);
   
   printf("Enter second number: "); //User input for second number
   scanf("%f", &num2);
   
   printf("\n");
   
   switch(menu) //switch statement for menu
   {
   case 1:
       result = num1 + num2; //Addition calculation
       break;
   case 2:
       result = num1 - num2; //Subtraction calculation
       break;
   case 3:
       result = num1 * num2; //Multiplication calculation
       break;
   case 4:
       result = num1 / num2; //Division calculation
       break;
   default:
       printf("Enter correct number e.g 1 - 4\n"); 
       exit(1);
   }
   printf("result is%.2f\n", result);
   

4. If you really want the operator in the output line, you can used a variable to hold this and set it in each branch of the switch statement.

5. I would allow the user to jump back to the beginning if they wanted. I would do this something like this:

   int main(int argc, char** argv)
   {
       float  num1, num2, result;
       char   choice, opcode;
       bool   bCont = true;  // flag to denote good input
   
       while(1)
       {
           puts("Enter a choice from the list below\n\n");
           puts("1 Addition\n"); /
           puts("2 Subtraction\n");
           puts("3 Multiplication\n");
           puts("4 Division\n\n");
           puts("q Quit\n\n");
   
           do
           {
               bCont = true;
               puts("Enter choice: "); //User input for the calculator menu
               scanf("%c", &choice);
   
               if(choice == 'q')
               {
                   exit(0);
               }
   
               if((choice < '1') || (choice > '4'))
               {
                   puts("enter 1 - 4 or q, please try again");
                   bCont = false;
               }
           } while (!bCont);
   
           printf("Enter first number: "); //User input for first number
           scanf("%f", &num1);
   
           printf("Enter second number: "); //User input for second number
           scanf("%f", &num2);
   
           switch(choice) //switch statement for menu
           {
               case '1':
                   result = num1 + num2; //Addition calculation
                   opcode = '+'
                   break;
   
               case '2':
                   result = num1 - num2; //Addition calculation
                   opcode = '-'
                   break;
   
               case '3':
                   result = num1 * num2; //Addition calculation
                   opcode = '*'
                   break;
   
               case '4':
                   if(0 != num2)
                        result = num1 / num2; //Addition calculation
                   else
                        result = Inf;
                   opcode = '/'
                   break;
           } // end of switch
            printf(printf("%.2f %c %.2f = %.2f\n", num1, opcode, num2, result);
       } // end of infinite while loop
   }
   

Just a note, notice that I chanced the menu choice from an integer to a character so I could accept both numeric and character values. Also look at the conditional I used: if((choice < '1') || (choice > '4')) this is based on the knowledge that the ASCII character code for the digits are consecutive and run from 0x30 to 0x39, so I can test for a valid input by determining if the character codes are in this range. This probably is not portable to systems using other encoding than ASCII.

Answer 3

• You should really consider making functions for each mathematical operation and keep the main() simple.

As an example:

float addition(float num1, float num2){

    return num1 + num2;
}

//or

void addition(float num1, float num2){

    float result = num1 + num2;
    printf("%0.2f", result);

}

In the second example, you create and destroy result each function call.

• Consider asking the user if they would like to return to the menu, in case they want to do multiple calculations.

• You don't need multiple printf statements for the menu options. Simply continuing as such is needed:

  printf("1. Addition\n2. Subtraction\n3. Multiplication\n4. Division\n\n");

Answer 4

Other answers have reviewed your code quite nicely, but all of them haven't mentioned one thing: Checking the return value of scanf.

For example, a piece of your code:

printf("Enter number: ");
scanf("%d", &menu);

does not check the return value of scanf. Change it to:

int retVal;              // Variable for capturing the return value
printf("Enter number: ");

while( (retVal = scanf("%d", &menu)) != 1  || menu < 1 || menu > 4)
{
    int c;
    while( (c = getchar()) != EOF && c != '\n'); // Clear the stdin

    printf("Invalid input; try again \n"); // Or use puts
}

Here, retVal captures the return value of scanf. It can be EOF, 0 or 1 in your case.

• Return value of 1 signifies that scanf was successful in getting a number from the user.
• Return value of 0 signifies that scanf was unsuccessful in getting a number from the user. This means that the user has inputted invalid data like characters instead of a number.
• Return value of -1 (EOF) signifies that scanf has encountered a reading error or EOF.

The body of the while loop will execute in case of invalid input. The first two lines of the body of this loop clears/flushes the stdin. This means that the standard input stream (stdin) is cleared of characters. This is done by reading and discarding all characters until EOF or \n (newline character).

This is done so that scanf does not go on reading the same invalid data ( when user enters characters or other invalid data) again and again and the loop from turning into an infinite loop.

The last line prints the error message and prompts the user to enter valid data.

You can use the same technique for the other scanfs in your code too.

Answer 5

First of all, create functions for the operators you use. For instance, if you are performing addition, write the function as:

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

Same for subtraction, multiplication and division.

Also, why do you want to get the values from the user for each case? Try to use function for it:

int get()
{int a;
  scanf("%d",&a);
  return a;
}

Call the function get() whenever you want to get the input from the user.