Three numbers calculator

Question

Edit: I made a new calculator after taking your input New Calculator

I made this three-function calculator. I have been learning C for a couple of months, and I learned C++ through the internet. This is my first "real" program in C++, it started really messy but I did a lot of refining and debuging and deleting unnecessary code and this is the final result. If you see any way to improve it or something I did wrong please tell me.

// Three numbers Calculator
#include "stdafx.h"
#include<iostream>
using namespace std;
int main()
{
    cout << "Enter action as # to exit program" << endl;
    cout << "Possible actions:+,-,*,/\n" << endl;
    while (1) //loop
    {
             long double num1, num2, num3, total;
         char action1, action2;
         cin >> num1 >> action1 >> num2 >> action2 >> num3;
         if (action1 == '/' && num2 == 0 || action2 == '/' && num3 == 0)
             cout << "You can't divide by zero" << endl;
         else if ((action2 == '*' || action2 == '/') && (action1 == '-' || action1 == '+'))  //action2 will be preformed before action1 (Order of operation)
         {
             switch (action2)  //I didn't include the options for '+' or '-' because the if statement requires action2 to be '*' or '/'.
             {
             case('/'):
                 total = num2 / num3;
                 break;
             case('*'):
                 total = num2*num3;
                 break;
             default:
                 cout << "Input not recognized";
                 break;
             }
             switch (action1)   //I didn't include the options for '*' or '/' because the if statement requires action1 to be '+' or '-'.
             {
             case('+'):
                 cout << num1 << "+" << num2 << action2 << num3 << "=" << total + num1;
                 break;
             case('-'):
                 cout << num1 << "-" << num2 << action2 << num3 << "=" << num1 - total;
                 break;
             default:
                 cout << "Input not recognized";
                 break;
             }
         }
         else //action1 will be performed before action2 (Order of operation)
         {
             switch (action1)
             {
             case('+'):
                 total = num1 + num2;
                 break;
             case('-'):
                 total = num1 - num2;
                 break;
             case('/'):
                 total = num1 / num2;
                 break;
             case('*'):
                 total = num1*num2;
                 break;
             case('#'):
                 system("PAUSE");
                 return 0;
                 break;
             default:
                 cout << "Input not recognized";
                 break;
             }
             switch (action2)
             {
             case('+'):
                 cout << num1 << action1 << num2 << "+" << num3 << "=" << total + num3;
                 break;
             case('-'):
                 cout << num1 << action1 << num2 << "-" << num3 << "=" << total - num3;
                 break;
             case('/'):
                  cout << num1 << action1 << num2 << "/" << num3 << "=" << total / num3;
                 break;      
             case('*'):
                 cout << num1 << action1 << num2 << "*" << num3 << "=" << total*num3;
                 break;
             case('#'):
                 system("PAUSE");
                 return 0;
                 break;
             default:
                 cout << "Input not recognized";
                 break;
             }
         }
         cout << "\n\n";
     }
}

---

Update:

Input: 7+8/2
Output: 7+8/2=11

Answer 1

I see a number of things that may help you improve your code.

Fix your formatting

The code as posted has inconsistent indenting which makes it hard to read. Choose a particular style and apply it consistently to make your code easier to read.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Validate the input

The code does not seem to be able to handle negative numbers as in the expression:

-3+2

If it's not intended to handle negative numbers, it would be good to let the user know. If it is, then there's a bug that should be fixed.

Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.

Don't use system("PAUSE")

There are two reasons not to use system("cls") or system("PAUSE"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named PAUSE or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example:

void pause() {
    getchar();
}

General portability

This code could be made portable if you omit the Windows-only include files #include "stdafx.h". If you must have stdafx.h, consider wrapping it so that the code is portable:

#ifdef WINDOWS
#include "stdafx.h"
#endif

Separate input, output and calculation

To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the program (which does not depend on the underlying OS). So for example, the main program might call a function to fetch and validate the input, a second function to actually perform the mathematical operations and then a third to emit the results.

Omit spurious parentheses

The argument to a case statement or return statement does not need parentheses, so instead of this:

 case('#'):

One could write this:

 case '#':

Consolidate strings

Right now, the string "Input not recognized" appears four times in the program. I'd recommend creating a const variable instead and then using that instead of repeating the string.

Answer 2

Leaving technicalities out, few general remarks regarding your code would be:

• Improve Style: try and add comments to your code to clarify its purpose and ease the readability. Ensure same code indentation.

• Increase modularity: try and separate the different phases/actions of your code in functions. For example: prompt message and input-reading, expression evaluation and result display. In this way it will be easier to spot a bug and correct it.

Few remarks on your question:

• Formulate clear question: for example, display an input-output example of your code and specify your areas of concern more specifically.

---

Now, about the calculator: a calculator does, roughly, the following things:

• reads input.

• parses input into tokens (operands and operations, functions, special characters); checks for expression validity.

• evaluates an expression: considering grammar, operation properties (precedence, arity, associativity).

• displays result.

The above could be used as guidelines to expand and further develop your calculator.

At this point, IMO, the best thing to do is to read some code (just research the subject and see what others have done), enrich your code-language vocabulary and use examples you find as benchmark / comparison.