I am in 10th grade and was trying to build a simple calculator in c. Although I've checked multiple times even though the code was correct as per the syntax, it is not showing the proper output. Multiplication gives an output of 0, and addition/subtraction gives output as the first number input. tried alternative methods, yet everything results in the same output

please try to troubleshoot the code explain me where am i going wrong.

#include <stdio.h>
#include <math.h>

//Creating a simple calculator in c

//add func
int add(int n1, int n2)
{
    int result = n1   n2;
    return(result);
}

//subtract func
int subtract(int n1, int n2)
{
    int result = n1 - n2;
    return(result);
}

//divide func
int divide(int n1, int n2)
{
    int result = n1 / n2;
    return(result);
}

//multiply func
int multiply(int n1, int n2)
{
    int result = n1 * n2;
    return(result);
}

int main()
{
    int num1, num2, ans;
    char op;//op means operator

    //use input
    printf("Enter first number: ");
    scanf("%d", &num1); //takes first number
    printf("\nEnter second number: ");
    scanf("%d", &num2); //takes second number
    printf("\nEnter operation: ");
    scanf("%s", &op); //takes an operator character like ' ', '-', '/', '*'

    //checks which operator has been inputted 
    if (op ==' ')
    {
        int ans = add(num1, num2);
        printf("Answer: %d", ans);
    }
    else if (op=='-')
    {
        ans = num1-num2;
        printf("Aswer: %d", ans);
    }
    else if (op=='/')
    {
        ans = divide(num1, num2);
        printf("Answer: %d",ans);
    }
    else if (op == '*')
    {
        ans = multiply(num1, num2);
        printf("Answer: %d",ans);
    }
    else //exits with error if input instead of   - / *
    {
    printf("Invalid operator!");
    }
    
    return 0;
}

CodePudding user response：

A few issues ...

1. As mentioned in the top comments, char op; will have UB (undefined behavior) when we do: scanf("%s",&op); because the %s adds a string terminator 0x00 at the end. So, op must be [at least] two chars in length.
2. A switch/case is better than an if/else "ladder".
3. We can simplify the code with just a single printf.

Here is the refactored code:

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

//Creating a simple calculator in c

//add func
int
add(int n1, int n2)
{
    int result = n1   n2;

    return (result);
}

//subtract func
int
subtract(int n1, int n2)
{
    int result = n1 - n2;

    return (result);
}

//divide func
int
divide(int n1, int n2)
{
    int result = n1 / n2;

    return (result);
}

//multiply func
int
multiply(int n1, int n2)
{
    int result = n1 * n2;

    return (result);
}

int
main()
{
    int num1, num2, ans;
#if 0
    char op;                            // op means operator
#else
    char op[10];                        // op means operator
#endif

    // user input
    printf("Enter first number: ");
    scanf("%d", &num1);

    printf("\nEnter second number: ");
    scanf("%d", &num2);

    // takes an operator character like ' ', '-', '/', '*'
    printf("\nEnter operation: ");
#if 0
    scanf("%s", &op);
#else
    scanf("%s", op);
#endif

    // checks which operator has been inputted
    switch (op[0]) {
    case ' ':
        ans = add(num1, num2);
        break;

    case '-':
        ans = subtract(num1,num2);
        break;

    case '/':
        ans = divide(num1, num2);
        break;

    case '*':
        ans = multiply(num1, num2);
        break;

    default:  // exits with error if input instead of   - / *
        printf("Invalid operator!");
        exit(1);
        break;
    }

    printf("Answer: %d\n", ans);

    return 0;
}

In the above code, I've used cpp conditionals to denote old vs. new code:

#if 0
// old code
#else
// new code
#endif

#if 1
// new code
#endif

Note: this can be cleaned up by running the file through unifdef -k

CodePudding user response：

Your primary issue is a misuse of scanf. (A reasonable argument can be made that any use of scanf is a misuse! You would do well to forget that scanf exists, since using it will likely hinder your learning of the language.) Parameters like this should really be taken from the command line arguments rather than the input stream, but the primary reason I'm adding this answer (it's really a comment, but it's hard to format comments well) is to demonstrate one method for reducing the duplication in your code. For each operator, you have boilerplate that doesn't need to exist. eg:

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

int add(int n1, int n2) { int result = n1   n2; return result; }
int subtract(int n1, int n2) { int result = n1 - n2; return result; }
int divide(int n1, int n2) { int result = n1 / n2; return result; }
int multiply(int n1, int n2) { int result = n1 * n2; return result; }

int
main(int argc, char **argv)
{
    int num1 = argc > 1 ? strtol(argv[1], NULL, 0) : 1;
    int op = argc > 2 ? argv[2][0] : ' ';
    int num2 = argc > 3 ? strtol(argv[3], NULL, 0) : 1;

    int (*f)(int, int);
    switch(op) {
    case ' ': f = add; break;
    case '-': f = subtract; break;
    case '/': f = divide; break;
    case '*': f = multiply; break;
    default:
        fprintf(stderr, "invalid operator\n");
        return 1;
    }
    int ans = f(num1, num2);
    printf("%d %c %d = %d\n", num1, op, num2, ans);
    return 0;
}

Note that returning int from a "divide" function is probably a terrible idea. It's not inherently wrong, but you may be surprised to find that divide(1, 2) returns 0. Make sure you understand that.