Recursive calculator in C

Question

This is a calculator I wrote for a student's project.

It still has a problem (one that I know of), where it can't handle inputs of the form 2*2*2... (more than one * or / sign). The case 2*2 was solved in hackish ways.

I'd appreciate any remarks or advice on the code and possibly how to solve that problem.

Relevant inputs are using +, -, *, /, (, ), etc.

EDIT: I fixed the problems listed above.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
void getInput(char * in) {
 printf("> ");
 fgets(in, 256, stdin);
}
int isLeftParantheses(char p) {
 if (p == '(') return 1;
 else return 0;
}
int isRightParantheses(char p) {
 if (p == ')') return 1;
 else return 0;
}
int isOperator(char p) {
 if (p == '+' || p == '-' || p == '*' || p == '/') return p;
 else return 0;
}
int performOperator(int a, int b, char p) {
 switch(p) {
 case '+': return a+b;
 case '-': return a-b;
 case '*': return a*b;
 case '/':
 if (b == 0) { printf("Can't divide by 0, aborting...\n"); exit(1); } 
 return a/b;
 }
 return 0;
}
char isDigit(char p) {
 if (p >= '0' && p <= '9') return 1;
 else return 0;
}
int charToDigit(char p) {
 if (p >= '0' && p <= '9') return p - '0';
 else return 0;
}
int isNumber(char * p) {
 while(*p) {
 if (!isDigit(*p)) return 0;
 p++;
 }
 return 1;
}
int len(char * p) {
 return int(strlen(p));
}
int numOfOperands(char * p) {
 int total = 0;
 while(*p) {
 if (isOperator(*p)) total++;
 p++;
 }
 return total+1;
}
int isMDGRoup(char * p) {
 while(*p) {
 if (!isDigit(*p) && *p != '/' && *p != '*')
 return 0;
 p++;
 }
 return 1;
}
int getLeftOperand(char * p, char * l) {
 // Grab the left operand in p, put it in l,
 //and return the index where it ends.
 int i = 0;
 // Operand is part of multi-*/ group
 if (isMDGRoup(p)) {
 while(1) {
 if (*p == '*' || *p == '/') break;
 l[i++] = *p++;
 }
 return i;
 }
 // Operand is in parantheses
 if(isLeftParantheses(*p)) {
 int LeftParantheses = 1;
 int RightParantheses= 0;
 p++;
 while(1) {
 if (isLeftParantheses(*p)) LeftParantheses++;
 if (isRightParantheses(*p)) RightParantheses++;
 if (isRightParantheses(*p) && LeftParantheses == RightParantheses)
 break;
 l[i++] = *p++;
 }
 // while (!isRightParantheses(*p)) {
 // l[i++] = *p++;
 // }
 l[i] = '0円';
 return i+2;
 }
 // Operand is a number
 while (1) {
 if (!isDigit(*p)) break;
 l[i++] = *p++;
 }
 l[i] = '0円';
 return i;
}
int getOperator(char * p, int index, char * op) {
 *op = p[index];
 return index + 1;
}
int getRightOperand(char * p, char * l) {
 // Grab the left operand in p, put it in l,
 //and return the index where it ends.
 while(*p && (isDigit(*p) || isOperator(*p) ||
 isLeftParantheses(*p) || isRightParantheses(*p))) {
 *l++ = *p++;
 }
 *l = '0円';
 return 0;
}
int isEmpty(char * p) {
 // Check if string/char is empty
 if (len(p) == 0) return 1;
 else return 0;
}
int calcExpression(char * p) {
 // if p = #: return atoi(p)
 //
 // else:
 // L = P.LeftSide
 // O = P.Op
 // R = P.RightSide
 // return PerformOp(calcExpression(L), calcExpression(R), O)
 // ACTUAL FUNCTION
 // if p is a number, return it
 if (isNumber(p)) return atoi(p);
 // Get Left, Right and Op from p.
 char leftOperand[256] = ""; char rightOperand[256]= "";
 char op;
 int leftOpIndex = getLeftOperand(p, leftOperand);
 int operatorIndex = getOperator(p, leftOpIndex, &op);
 int rightOpIndex = getRightOperand(p+operatorIndex, rightOperand);
 printf("%s, %c, %s", leftOperand, op, rightOperand);
 getchar();
 if (isEmpty(rightOperand)) return calcExpression(leftOperand);
 return performOperator(
 calcExpression(leftOperand),
 calcExpression(rightOperand),
 op
 );
}
int main()
{
 char in[256];
 while(1) {
 // Read input from user
 getInput(in);
 if (strncmp(in, "quit", 4) == 0) break;
 // Perform calculations
 int result = calcExpression(in);
 printf("%d\n", result);
 }
}

• syb0rg: This codes doesn't compile for multiple reasons, so I am led to believe this code doesn't work.
• palacsint: @syb0rg: gcc can't compile it, but it compiles with g++.
• hXJaXNJxPrYRBxXiCzWc: Not sure I understand, works fine for me, along with the limitations specified above.
• amon: It's not clear if this is supposed to be C or C++. Could you make the language absolutely clear, and tell us your compiler setup (e.g. which compiler and what options you are using)
• hXJaXNJxPrYRBxXiCzWc: Honestly, I don't know. Sorry. I intended for this to be C only, could you tell me what part is C++? Also, palacsint says he successfully compiled it using g++, so I assume that is what I'm using (Sublime Text 2). I'm sorry you can't run it for some reason, my question was mostly about the logic really. Thanks regardless.

Answer 1

Things you did well on:

• Organization

• Use of comments

• Using switch statements instead of multiple if-elses.

Things that could be improved:

Compilation

• I couldn't compile this program with this method written the way it is.

  int len(char * p) {
   return int(strlen(p));
  }
  

  I'm going to assume that is due to the issues of you compiling it with g++, and me having a strict compiler. My compiler thinks that this code is trying to make some sort of function call instead of parsing the string length to an int. Put the parentheses around the int instead, and it will work just fine.

  int len(char * p)
  {
   return (int) strlen(p);
  }
  

Refactoring

• You could simplify down some of your loops.

  int isMDGRoup(char * p) {
   while(*p) {
   if (!isDigit(*p) && *p != '/' && *p != '*')
   return 0;
   p++;
   }
   return 1;
  }
  

  Use a for loop here instead.

  int isMDGRoup(char *p)
  {
   for(; *p; p++)
   {
   if (!isDigit(*p) && *p != '/' && *p != '*') return 0;
   }
   return 1;
  }
  

Syntax

• When you return variables that are performing operations on the same line as the return, surround them with parenthesis.

  return (i+2);
  

• You use some magic numbers.

  char in[256];
  

  I see you use the number 256 in some other places as well. Pull out that value into a #defined variable.

  #define BUFLEN 256
  

• Use puts() instead of printf() when you aren't formatting strings.

• Use a default in your switch.

  default:
  puts("Error message.");
  break;
  

Error handling:

• It is more common to return 0 (or some error indicator) rather than to exit(0). Both will call the registered atexit handlers and will cause program termination though. You have both spread throughout your code. Choose one to be consistent. Also, you should return different values for different errors, so you can pinpoint where something goes wrong in the future.

• Your program can't handle the input of non-whole numbers.

  > 7.7*6
  7, ., 7*6
  7, *, 6
  0
  >
  

• Your program can't handle malformed input.

  > 7a*6
  7, a, *6
  , *, 6
  0
  >
  

• Your program can't handle negative numbers properly

  > -5+6
  , -, 5+6
  5, +, 6
  -11
  >
  

• Your program can't handle whitespace.

  > 5 * 6
  5, , *
  , *, 
  0
  > 
  

• Your program doesn't handle no input very well.

  > 
  , 
  , 
  0
  > 
  

• Your program can't handle numbers larger than the max size of an int.

  > 99999999999999*99999999999999
  9999999999999999999, *, 99999999999999
  -276447231
  >