I'm not understanding how my C-calculator is working

  • #1
Eclair_de_XII
1,068
90
TL;DR Summary
I am tasked to write a calculator. It will accept user input in the form: (operator) (operand). The range of operations include: set (s), end (e), and the usual arithmetic operations.
C:
#include <stdio.h>

void calculate(float *accumulator,char operator,float number){
    switch(operator){
    case 's':
        *accumulator=number; break;
    case '+':
        *accumulator+=number; break;
    case '-':
        *accumulator-=number; break;
    case '*':
        *accumulator*=number; break;
    case '/':
        if (number != 0)
            *accumulator/=number;
        else
            printf("Stop dividing by zero!\n");
        break;
    }
}

int main(void){
    float number,*accumulator;
    char operator='\0';
    printf("Begin Calculations\n");

    while (operator != 'e') {
        scanf("%c %f",&operator,&number);
        calculate(accumulator,operator,number);
        printf("= %.6f\n\n",*accumulator);
    }

    return 0;
}

When I run this code, it seems to read the print statement twice on subsequent calculations. Moreover, when I need to add or subtract things, I will need to type in the appropriate operator symbols twice. I do not know why that is.

EDIT: For some reason, it seems to function properly if I transpose the order of the scanf arguments; but it does go on in an infinite loop if I give erroneous input. But this does not answer my question.
 
Last edited:

Answers and Replies

  • #2
sysprog
2,611
1,783
It looks to me like the code decides whether to run the while loop before it reads the next scanf value, so when the while loop is run due to a non-'e' value for 'operator', it runs the scanf, and if you enter 'e', it still runs another calculate and printf afterward. To eliminate that, you could insert another test for 'e' after the scanf and before the calculate and printf in that loop ##-## if (operator != 'e') {. . .} ##-## but it seems inelegant to test for that twice ##-## can you see a way to eliminate calculate and printf being run after the scanf gets an 'e', without a repeated such test?

Here's a link to a page with a sample C++ program for a simple calculator: https://www.programiz.com/cpp-programming/examples/calculator-switch-case

A more sophisticated C++ and C# calculator program is available for your perusal ##-## MS has released its Windows Calculator code under the MIT License: https://github.com/Microsoft/calculator
 
Last edited:
  • #3
36,854
8,888
The main thing wrong with this code is the declaration of accumulator as an uninitialized pointer. That means that after the first call to scanf(), the program will try to store the input number at some random location. When I attempt to compile your code using VS 2017, the compiler generates an uninitialized local variable error.

To fix this problem, I made this change to main():
C:
int main(void) {
    float number, accumulator = 0.0f;   // accumulator is no longer a pointer, and it's now initialized.
    char operator='\0';
    printf("Begin Calculations\n");

    while (operator != 'e') {
        scanf("%c %f", &operator,&number);
        calculate(&accumulator, operator,number);
        printf("= %.6f\n\n", accumulator);       // I also changed this line. accumulator is no longer dereferenced.
    }
    return 0;
}

To address the problem you were having, it's usually a very bad idea to use scanf() to input to more than one variable, not to mention that the program does not issue a prompt that tells the user what should be entered. The use of scanf() also has been strongly discouraged in Visual Studio for many years, as it is prone to being used by hackers in buffer overrun attacks.

The problem you are seeing arises directly from your use of scanf() to input a char and a number. In the first call to scanf(), if I enter, say, +[space]2.2[Enter], operator will be set to '+' and number will be set to 2.2. The problem is that the input buffer still has a character in it; namely, the Enter character I typed(actually ASCII 10, '\n').

A workaround for this problem is to get rid of the final character in the input buffer before the next call to scanf().
C:
while (operator != 'e') {
        scanf("%c", &operator);
        scanf("%f", &number);
        scanf("%c", &junk);                           // Gobble up the final newline character, '\n'
        calculate(&accumulator, operator,number);
        printf("= %.6f\n\n", accumulator);             
    }
Of course, you need to declare junk...
 
  • #4
36,854
8,888
Here's something that works a bit better than what you had. Main changes are:
  • addition of a default case -- always a good practice in a switch statement.
  • separate input statements for operator and number, each with its own prompt.
  • change from scanf() to scanf_s(). Visual Studio, which is what I use, has deprecated the use of scanf() and related input functions for at least the past 10 years, and recommends using scanf_s() in place of scanf(). These are MSFT-only functions, and AFAIK, other compilers don't support these more secure input functions.
  • change the while loop to run forever ( while(1) ), but exiting the loop if the input operator is 'e'.
  • extract and ignore any final new-line characters ( scanf_s("%c", &junk, 1); ).

C:
#include <stdio.h>

void calculate(float *accumulator, char operator, float number) {
    switch (operator) {
    case 's':
        *accumulator = number; break;
    case '+':
        *accumulator += number; break;
    case '-':
        *accumulator -= number; break;
    case '*':
        *accumulator *= number; break;
    case '/':
        if (number != 0)
            *accumulator /= number;
        else
            printf("Stop dividing by zero!\n");
        break;
    default: printf("Invalid operator was entered!\n"); 
    }
}

int main(void) {
    float number, accumulator = 0.0f;
    char operator = 0, junk = 0;
    int ret;
    printf("Begin Calculations\n");

    while (1) {
        printf("Enter operator: ");
        scanf_s("%c", &operator, 1);
        if (operator == 'e') break;
        printf("Enter number: ");
        scanf_s("%f", &number);
        scanf_s("%c", &junk, 1);
        calculate(&accumulator, operator,number);
        printf("= %.6f\n\n", accumulator);              
    }
    return 0;
}
 
Last edited:
  • #5
Eclair_de_XII
1,068
90
The main thing wrong with this code is the declaration of accumulator as an uninitialized pointer. That means that after the first call to scanf(), the program will try to store the input number at some random location.

The problem you are seeing arises directly from your use of scanf() to input a char and a number. In the first call to scanf(), if I enter, say, +[space]2.2[Enter], operator will be set to '+' and number will be set to 2.2. The problem is that the input buffer still has a character in it; namely, the Enter character I typed(actually ASCII 10, '\n').

That is helpful to know.

To address the problem you were having, it's usually a very bad idea to use scanf() to input to more than one variable, not to mention that the program does not issue a prompt that tells the user what should be entered

The tip is much appreciated, also.
 

Suggested for: I'm not understanding how my C-calculator is working

Replies
12
Views
256
Replies
9
Views
879
  • Last Post
Replies
1
Views
352
Replies
3
Views
604
  • Last Post
Replies
4
Views
707
Replies
14
Views
761
  • Last Post
Replies
3
Views
351
  • Last Post
2
Replies
50
Views
2K
Replies
13
Views
1K
Top