### Calculator Help!

Im trying to make a basic calculator that takes in an expression. Im not really sure where im going wrong so if someone could please take a look at my code and give a few suggestions.

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121`` ``````#include #include #include #include #include #include #include using namespace std; bool precedence(stack& operators); double read_and_evaluate(string exp); void evaluate_stack_tops(stack& numbers, stack& operators); stack numbers; stack operators; int main(){ double answer; string exp; cout << "Enter in an experssion: " << endl; cin >> exp; answer = read_and_evaluate(exp); cout << "That evaluates to: " << answer << endl; return 0; } bool precedence(char &a, char &b){ b= operators.top(); operators.pop(); a=operators.top(); if(b=='~') return false; else if(a=='(') return false; else if(a=='~') return true; else if(a=='\$') return false; else if(b== '^') return false; else if(a== '+' || a=='-') return false; else if(b=='*' || b=='/' || b=='%') return false; else return true; } double read_and_evaluate (string exp){ for(int i =0; i < exp.length(); i++){ if(isdigit(exp[i])){ numbers.push(exp[i]); cout << "these are the numbers being used: " << exp[i] << endl; } else if (exp[i] == '+'|| exp[i] == '*' || exp[i] == '/' || exp[i] == '%' || exp[i] == '^' << exp[i] == '('){ operators.push(exp[i]); cout << "This is an operator : " << exp[i] << endl; } else if(exp[i] == '-'){ if(exp[i-1] == '('){ exp[i] = '~'; cout << "This is an expression " << exp[i] << endl; operators.push(exp[i]); } else{ cout << "this is an expression: " << exp[i] << endl; operators.push(exp[i]); } } else if(exp[i] == ')'){ while(operators.top() !='(') evaluate_stack_tops(numbers, operators); } } return numbers.top(); } void evaluate_stack_tops(stack& numbers, stack& operators){ double num2; double num1; num2 = numbers.top(); numbers.pop(); num1 = numbers.top(); numbers.pop(); switch(operators.top()){ case '+': numbers.push(num1+num2); break; case '-': numbers.push(num1-num2); break; case '*': numbers.push(num1*num2); break; case '/': numbers.push(num1/num2); break; case '^': numbers.push(pow(num1,num2)); break; case '%': numbers.push(fmod(num1,num2)); break; case '~': numbers.push(num2 * -1); break; } operators.pop(); }``````
Without looking too hard at your code, I think the main problem is that you're not taking precedence into consideration when pushing operators into your stack. Regardless of the particular operators in an expression, the program will always apply them in the same order. The end result being that 1+2*3 ≠ 2*3+1.
Another subtlety is that ^ typically associates to the right. Meaning a^b^c == a^(b^c).

precedence() doesn't make much sense to me.

There's a stray << on line 60.
I was just trying to get it to do a basic expression first like (3+4) but it wasnt working so i decided to comment all of that out until i figured out the rest. I have found out that i need to convert my exp to double using strtod().
It will be better to use if else loop then switch case. Your code makes confused. I do not have a mean "precedence" ?
really it is so confusing.. Change the codes...
@OP : Your code style :
 if(b=='~') return false; else if(a=='(') return false; else if(a=='~') return true; else if(a=='\$') return false; else if(b== '^') return false; else if(a== '+' || a=='-') return false; else if(b=='*' || b=='/' || b=='%') return false; else return true;

Could be changed to :
 ``12345678910`` `````` if(b == ' ~ ')return false; if(a == ' ( ')return false; if(a == ' \$ ')return false; if(b == ' ^ ')return false; if(a == ' + ' || a == ' - ')return false; if(b == ' * ' || b == ' / ' || b == ' % ')return false; return true;``````

Each comparison expression has a "return" command, so no need to add "else"... This make your code harder to read - understand...
Last edited on
 switch(operators.top()){ case '+': numbers.push(num1+num2); break; case '-': numbers.push(num1-num2); break; case '*': numbers.push(num1*num2); break; case '/': numbers.push(num1/num2); break; case '^': numbers.push(pow(num1,num2)); break; case '%': numbers.push(fmod(num1,num2)); break; case '~': numbers.push(num2 * -1); break; }

 ``1234567891011`` `````` switch(operators.top()){ case '+': numbers.push(num1 + num2);break; case '-': numbers.push(num1 - num2);break; case '*': numbers.push(num1 * num2);break; case '/': numbers.push(num1 / num2);break; case '^': numbers.push(pow(num1,num2));break; case '%': numbers.push(fmod(num1,num2));break; case '~': numbers.push(num2 * -1);break; }``````
@Jackson Marie - It's not good practice to have >1 return expression. Much better to declare a return variable of the correct type and set that in your code. Neither will your variables compare if you have spaces:

 ``12`` ``````char b = 'b'; if (b == ' b ')``````

the above will evaluate to false not true as you expect.
 if(b == ' ~ ')return false; if(a == ' ( ')return false; if(a == ' \$ ')return false; if(b == ' ^ ')return false; if(a == ' + ' || a == ' - ')return false; if(b == ' * ' || b == ' / ' || b == ' % ')return false; return true;

 ``123456`` `````` if(a == '+' || a == '-'|| a == '(' || a == '\$')return false; if(b == '*' || b == '/' || b == '%' || b == '~' || b == '^')return false; return true;``````
Last edited on
Topic archived. No new replies allowed.