How to optimize C code with else if conditions

Question

I'm learning some basics of C (I'm actually doing Harvard's cs50x) and I wrote this code:

#include <cs50.h>
#include <stdio.h>

int main(void) {
    while(0 == 0) {
        printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
    
        int a = get_int("Choose one option: ");
        
        if (a == 1) {
            int x = get_int("x: ");
            int y = get_int("y: ");
            int sum = x + y;
            int sub = x - y;
            int division = x/y;
            int mul = x*y;
            printf("Result = %i\n\n", sum);
        }
        else if (a == 2) {
            int x = get_int("x: ");
            int y = get_int("y: ");
            int sum = x + y;
            int sub = x - y;
            int division = x/y;
            int mul = x*y;
            printf("Result = %i\n\n", sub);
        }
        else if (a == 3) {
            int x = get_int("x: ");
            int y = get_int("y: ");
            int sum = x + y;
            int sub = x - y;
            int division = x/y;
            int mul = x*y;
            printf("Result = %i\n\n", mul);
        }
        else if (a == 4) {
            int x = get_int("x: ");
            int y = get_int("y: ");
            int sum = x + y;
            int sub = x - y;
            int division = x/y;
            int mul = x*y;
            printf("Result = %i\n\n", division);
        }
        else if (a == 5) {
            printf("Alright! See you soon!\n");
            break;
        }
        else {
            printf("Invalid Input\n");
            break;
        }
    }
}

It works exactly as I want it to, but I feel like it could be written in a better way.
I really don't like all that repetition of int variables and I think that it could be optimized someway but don't know how.

Answer 1

All of the operations except for the "QUIT" and "invalid" operations have the lines

int x = get_int("x: ");
int y = get_int("y: ");

in common. Therefore, it would make sense to move these two lines outside the if...else if chain, so that you only have to write them once. However, the cases of "QUIT" and "invalid" must be handled beforehand, so that the user is not prompted to enter these two operands if he wants to quit or the input is invalid.

Also, you are always performing all 4 arithmetic operations (addition, subtraction, multiplication and division), although it would only be necessary to calculate the one that the user selected.

Additionally, I doubt that these lines are correct:

else {
    printf("Invalid Input\n");
    break;
}

If the user enters invalid input, you probably want to reprompt the user for input by restarting the loop, instead of exiting the program.

Instead of using a long if...else if chain, you can use a switch statement.

The line

while (0==0){

is technically correct, however it is very uncommon to write it that way.

In order to create an infinite loop, one would normally either write while (1) or for (;;), the latter being the more traditional syntax.

Here is a program which incorporates the mentioned improvements:

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

int main( void )
{
    for (;;)
    {
        printf(
            "1. SUM\n"
            "2. SUBSTRACTION\n"
            "3. MULTIPLICATION\n"
            "4. DIVISION\n"
            "5. QUIT\n"
        );
    
        int a = get_int( "Choose one option: " );

        if ( a == 5 )
        {
            printf( "Alright! See you soon!\n" );

            //break out of infinite loop
            break;
        }

        //verify that input is between 1 and 4
        if ( ! ( 1 <= a && a <= 4 ) )
        {
            printf( "Invalid Input\n\n" );

            //restart loop so that user gets reprompted for input
            continue;
        }

        int x = get_int( "x: " );
        int y = get_int( "y: " );

        switch ( a )
        {
            case 1:
                printf( "Result = %i\n\n", x + y );
                break;
            case 2:
                printf( "Result = %i\n\n", x - y );
                break;
            case 3:
                printf( "Result = %i\n\n", x * y );
                break;
            case 4:
                printf( "Result = %i\n\n", x / y );
                break;
            default:
                //these lines should never be reached
                fprintf( stderr, "internal program error!" );
                exit( EXIT_FAILURE );
        }
    }
}

Answer 2

@AndreasWenzel's answer is already quite good, using conditional guards and control flow tools like continue and break.

As an alternative, you can assign to a result variable in the switch, and then do a single printf for output.

int main(void) {
    while (1) {
        printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
    
        int a = get_int("Choose one option: ");
        
        if (a == 5) {
            printf("Alright! See you soon!\n");
            return 0;
        } 
        else if (a < 1 || a > 4) {
            printf("Invalid Input\n");
            continue;
        }

        int x = get_int("x: ");
        int y = get_int("y: ");
        int result;

        switch (a) {
            case 1:
            result = x + y;
            break;

            case 2:
            result = x - y;
            break;

            case 3:
            result = x * y;
            break;
 
            case 4:
            result = x / y;
            break;
        }

        printf("Result = %i\n", result);
    }

    return 0;
}

As an alternative, we could check for good input and the quit choice as before, but use an array of function pointers to implement this quite concisely.

int add(int a, int b) { return a + b; }
int sub(int a, int b) { return a - b; }
int mul(int a, int b) { return a * b; }
int div(int a, int b) { return a / b; }

typedef int(*op)(int, int);

int main(void) {
    op ops[] = { add, sub, mul, div };    

    while(0 == 0) {
        printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
    
        int a = get_int("Choose one option: ");       

        if (a == 5) {
            printf("Alright! See you soon!\n");
            return 0;
        } 
        else if (a < 1 || a > 4) {
            printf("Invalid Input\n");
            continue;
        }

        int x = get_int("x: ");
        int y = get_int("y: ");
        int result = ops[a - 1](x, y);

        printf("Result = %i\n", result);
    }

    return 0;
}

Going a step further, we can break out the selection of an operation to perform into a separate function with all of the required input error handling, and group function pointers and menu options into structs.

typedef int(*op)(int, int);

typedef struct {
    op operation;
    char *menu_option;
} option;

option get_option(char *prompt, char * out_of_range_msg, char *out_of_tries_msg, 
                  option *ops, size_t n, int tries,
                  int add_quit_option, char *quit_option_text, char *quit_msg) {
    while (tries--) {
        for (size_t i = 0; i < n; ++i) {
            printf("%2d: %s\n", i + 1, ops[i].menu_option);
        } 

        if (add_quit_option) {
            printf("%2d: %s\n", n + 1, quit_option_text);
        }

        int opt = get_int(prompt);

        if (opt < 1 || opt > (add_quit_option ? n + 1 : n)) {
            printf("%s\n", out_of_range_msg);
            continue;
        }

        if (add_quit_option && opt == n + 1) {
            printf("%s\n", quit_msg);
            exit(EXIT_SUCCESS);
        }

        return ops[opt - 1];
    }

    printf(out_of_tries_msg);
    exit(EXIT_FAILURE);
}

Now main becomes cleaner still, and we see how easy it can become to add an additional operation.

int main(void) {
    option ops[] = { 
        {addop, "Sum"}, {subop, "Subtraction"}, 
        {mulop, "Multiplication"}, {divop, "Division"}, 
        {modop, "Modulo"}
    };    

    while (1) {
        option op = get_option("Choose One option: ", "Invalid input", "Too many mistakes!", 
                               ops, 5, 10,
                               1, "Quit", "Alright! See you soon!");

        int x = get_int("x: ");
        int y = get_int("y: ");
     
        printf("Result: %i\n", op.operation(x, y));
    }

    return 0;
}

Answer 3

To advance your code, you can create functions of the operations and then utilize them either with if...else ladder, or switch...case statements. You can look through the following code and ask any questions in mind. Please note that I am not a professional C programmer, and this is how I modified your code. I believe there will be much better ways of modifying your program.

#include <stdio.h>

int main(void) {
    
        int a;
    
        float x;
        float y;
        
        float result;
        float my_remainder;
        
        // Let's create functions first
        float my_sum(float x, float y)        // function for addition
        {
            result = x + y;
        };
        
        float my_sub(int x, int y)           // function for subtraction
        {
            result = x - y;
        };
        
        float my_mul(int x, int y)           // function for multiplication
        {
            result = x * y;
        };
        
        float my_div(int x, int y)          // function for divison
        {
            result = x / y;
            my_remainder = x % y;
        };
        
    while(1){
        start:
        printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
        
        choose:                     // let's choose an option, but can choose only one of the mentioned ones
        printf("choose one option: ");
        scanf("%d", &a);
        
        switch(a)                  // check if "a" is not equal to either one of the mentioned numbers, then go back to 'choose' to ask for the right one
        {
            case 1: case 2: case 3: case 4: case 5: goto letsgo;
            
            default:
            printf("Invalid Input. Please choose an option from the given ones:\n\n"); 
            goto start;
        };
        letsgo:
        if(a == 5)                  // firstly, need to check QUIT condition, if QUIT is selected then the while loop stops (the process ends)
        {
            printf("\nAlright! See you soon!\n");
            //return 0;
            break;
        }
        
        else                        // if QUIT is not selected, it means one of the operations should be done
        {                           // so let's get the numbers for corresponding calculation
        printf("Please enter first number: ");
        scanf("%f", &x);
        printf("Please enter second number: ");
        scanf("%f", &y);
        
        if(a == 1)          // addition
        {
            my_sum(x,y);
            printf("Result = %.2f", result);        // %.2f <-- "2" represents the number of digits after the decimal separator. 
           return 0;                                // e.g., 1.4 + 1.2 = 2.60, however, if it was %.4f, then: 1.4 + 1.2 = 2.6000
        }
        else if(a == 2)     // subtraction
        {
            my_sub(x,y);
            printf("Result %.2f", result);
            return 0;
        }
        else if(a == 3)     // multiplication
        {
            my_mul(x,y);
            printf("Result %.2f", result);
            return 0;
        }
        else if(a == 4)     // division
        {
            my_div(x,y);
            printf("Result %.2f", result);
            if(my_remainder != 0)           // check if remainder is not equal to 0, then print, otherwise, don't print
            {
            printf("\nRemainder: %.2f", my_remainder);
            }
            return 0;
        }
        
        }
        
    }
        
}

Answer 4

this is the full code version

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

int main() {
int choose = 0, num1, num2, work;
float div;

while(choose!=5){
    
    printf(" 1. SUM\n 2. SUBSTRACTION\n 3. MULTIPLICATION\n 4. DIVISION\n 5. QUIT\n");
    printf("Choose an option... \n");
    scanf("%d", &choose);

    printf("\nInsert two numbers: \n");
    scanf("%d", &num1);
    scanf("%d", &num2);
    
    switch(choose){
        case 1:
            work = num1 + num2;
            printf("Sum: %d", work);
            break; //you can use it to terminate your case switch instruction
        
        case 2:
            work = num1 - num2;
            printf("Sub: %d", work);
            break;

        case 3:
            work = num1 * num2;
            printf("Multiplication: %d", work);
            break;

        case 4:
            div = num1 / num2;
            printf("Division: %f", div);
            break;
        
        case 5:
            choose = 5;
            break;
        
        default:
            printf("Error / invalid input");
            break;
    }
}

}