Question

Basically I am writing a simple command line calculator that implements an accumulator. I feel like this code is logically structured correct and I don't understand why it freezes for about 3 seconds before going into an infinite loop of print statements. Any help is appreciated.

void mycalc() {
  printf("Begin Calculations\n\n");
  printf("Initialize your Accumulator with data of the form \"number\" \"S\" which\
  sets the Accumulator to the value of your number\n");

  /* Initialize Variables */
  float accumulator, num;
  char op;

  /* Ask for input */
  scanf("%f %c\n", &num, &op);
  while (op != 'E') {
    if(op == 'S' || op == 's'){
      accumulator = num;
      printf("Value in the Accumulator = %f\n", accumulator);
    } else if(op == '+'){
      accumulator = accumulator + num;
      printf("Value in the Accumulator = %f\n", accumulator);
    } else if(op == '*'){
     accumulator = accumulator * num;
      printf("Value in the Accumulator = %f\n", accumulator);
    } else if(op == '/'){
      if (num == 0) {
          printf("Can not divide by 0.\n");
      } else {
          accumulator = accumulator / num;
          printf("Value in the Accumulator = %f\n", accumulator);
      }
    } else if(op == '-'){
      accumulator = accumulator - num;
      printf("Value in the Accumulator = %f\n", accumulator);
    } else if(op == 'E' || op == 'e'){
      printf("Value in the Accumulator = %f\n", accumulator);
      break;
    } else {
      printf("Unknown operator. \n");
    }
    scanf("%f %c\n", &num, &op);
  }
}

Would it be better to use the while(1) technique instead? Any and all help is appreciated! Thanks!

Was it helpful?

Solution

Code does not handle bad input well.

In two places scanf("%f %c\n", &num, &op) has trouble with if non-numeric input is entered. The scanf() fails, so num and op retain their old values. The operation based on op occurs again and the next scanf() tries again with the same data.

The "%f %c\n" in 2 places is misleading in that the \n performs differently than OP expectations. Change to

scanf("%f %c", &num, &op);

Instead of using scanf(), recommend using

char buf[100];
if (fgets(buf, sizeof(buf), stdin) == NULL) {
  exit(-1); // handle EOF or error
}
if (sscanf(buf, "%f %c", &num, &op) != 2) {
  exit(-1); // syntax error.
}

Alternatively one could use the following. Bad input will eventually get consumed but not so readily.

if (2 != scanf(" %c %f", &op, &num)) {
  ; // syntax error.
}

Other issues: accumulator is not initialized

float accumulator = 0.0;
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top