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

  • Context:
  • Thread starter Thread starter Eclair_de_XII
  • Start date Start date
Click For Summary

Discussion Overview

The discussion revolves around issues encountered in a C calculator program, focusing on input handling, pointer initialization, and the behavior of the scanf function. Participants explore potential bugs, suggest corrections, and share alternative approaches to improve the code's functionality.

Discussion Character

  • Technical explanation
  • Debate/contested
  • Mathematical reasoning

Main Points Raised

  • One participant notes that the accumulator is declared as an uninitialized pointer, which could lead to undefined behavior when storing input values.
  • Another participant suggests that the while loop runs the scanf function before checking the operator, causing an additional calculation and print statement to execute even when 'e' is entered.
  • A different approach is proposed to separate the input statements for the operator and number, which may help avoid issues with leftover characters in the input buffer.
  • One participant emphasizes the importance of adding a default case in the switch statement for better error handling.
  • There is a suggestion to use scanf_s instead of scanf, as the former is recommended by Visual Studio for security reasons.
  • Concerns are raised about the use of scanf for multiple variable inputs, which can lead to complications if the input is not formatted correctly.

Areas of Agreement / Disagreement

Participants express varying opinions on the best practices for input handling in C, with some agreeing on the need for better error handling and input separation, while others focus on the specific issues related to pointer initialization and scanf usage. No consensus is reached on a single solution.

Contextual Notes

Limitations include the potential for buffer overrun issues with scanf, the need for proper initialization of pointers, and the handling of newline characters left in the input buffer after reading input. These factors contribute to the complexity of the program's input handling.

Eclair_de_XII
Messages
1,082
Reaction score
91
TL;DR
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:
Technology news on Phys.org
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:
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...
 
  • Like
Likes   Reactions: sysprog
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:
  • Like
Likes   Reactions: sysprog
Mark44 said:
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.

Mark44 said:
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.

Mark44 said:
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.
 

Similar threads

Replies
7
Views
2K
  • · Replies 20 ·
Replies
20
Views
2K
  • · Replies 9 ·
Replies
9
Views
2K
  • · Replies 0 ·
Replies
0
Views
2K
Replies
14
Views
4K
  • · Replies 4 ·
Replies
4
Views
1K
  • · Replies 3 ·
Replies
3
Views
1K
  • · Replies 1 ·
Replies
1
Views
2K
  • · Replies 6 ·
Replies
6
Views
3K
Replies
47
Views
5K