Console calculator

Question

I am a newbie programmer learning some C++. One of the exercises in my book asked me to write a basic console calculator that takes 2 numbers and an operation as input. I have done so and the program compiles (both with clang++ and g++) and runs fine.

I would like to know if:

• I am making mistakes in my code that the compiler is allowing me to get away with
• If my code can be made more resource efficient and/or shorter in length

#include <iostream>
using namespace std;
int main()
{
char operation;
double first_number;
double second_number;
double solution = 0;
cout << "This is a basic calculator program, enter the first number.\n";
cout << "First number:";
cin >> first_number;
cout << "Enter second number:";
cin >> second_number;
cout << "\nEnter an operation to perform, choose one from this list: +,-,/,*\n";
cout << "Enter your operation:";
cin >> operation;
cin.ignore();
if (operation != '+' && operation != '-'&& operation != '/'&& operation != '*')
 {
 cout << "\nInvalid operation! Aborting!";
 cout << "\nPress Enter to exit.";
 cin.get();
 return 1;
 }//end if
if (operation == '+')
 {
 solution = first_number + second_number;
 cout << "\nYour answer is: " << solution << "\nPress Enter to exit.\n";
 cin.get();
 return 0;
 }//end if
if (operation == '-')
 {
 solution = first_number - second_number;
 cout << "\nYour answer is: " << solution << "\nPress Enter to exit.\n";
 cin.get();
 return 0;
 }//end if
if (operation == '*')
 {
 solution = first_number * second_number;
 cout << "\nYour answer is: " << solution << "\nPress Enter to exit.\n";
 cin.get();
 return 0;
 }//end if
if (operation == '/')
 {
 if ( second_number == 0 )
 {
 cout << "\nYou can't divide by zero! Aborting!";
 cout << "\nPress Enter to exit.";
 cin.get();
 return 2;
 }//end if
 solution = first_number / second_number;
 cout << "\nYour answer is: " << solution << "\nPress Enter to exit.\n";
 cin.get();
 return 0;
 }//end if
return 0;
}//end main

• nhahtdh: You can use enum to indicate the status, and move all the duplicate code to output and return to one place.
• Keith Flower: Great, it's always a nice approach to get code running first, then refine it. For starters, you might investigate use of a 'switch' statement instead of your multiple 'if' statements. Also, take a look at code you duplicate in several places. Such code might make up a function.
• reuben: If you'd like to see if the compiler is "letting you get away with things", you can try increasing the strictness / warning level on compiler (most likely via command-line options)...
• balki: since the operators are char, you can use a switch case instead of if's

Answer 1

I'd try something like this. This is more robust as it checks for a 0 as the denominator and an invalid operation character.

Checking the former before attempting a calculation would be best, though you can still have it ask again for a proper denominator instead of just terminating the program right away. I did the latter here anyway for simplicity.

Checking the latter would involve either terminating the program right away or by throwing an exception since calculate() must return something (throw would replace return here, but should only be used in case of an error).

#include <cstdlib> // EXIT_FAILURE
#include <iostream>
#include <stdexcept> // std::logic_error
float calculate(const char operation, const float left, const float right)
{
 switch (operation)
 {
 case '+': return left + right;
 case '-': return left - right;
 case '*': return left * right;
 case '/': return left / right;
 default: throw std::logic_error("unsupported operator");
 }
}
int main()
{
 std::cout << "Enter your two numbers: \n\n";
 float left, right;
 std::cin >> left >> right;
 std::cout << "\nEnter your operation (+, -, *, /): ";
 char operation;
 std::cin >> operation;
 // terminate right away if dividing by zero
 if (operation == '/' && right == 0)
 {
 std::cerr << "Cannot divide by 0";
 return EXIT_FAILURE;
 }
 float result;
 // attempt the calculation (will throw if failed)
 try
 {
 result = calculate(operation, left, right);
 }
 // if it fails - catch exception, display it, then terminate
 catch (std::logic_error const& e)
 {
 std::cerr << "Error: " << e.what();
 return EXIT_FAILURE;
 }
 std::cout << "\nResult = " << result;
}

• Lstor: #include <cstdlib> for EXIT_FAILURE. Rename the number variables to left and right or something. And throw an exception in the default clause instead of silently swallowing an error. Apart from that, much cleaner than the other approaches.
• Jamal: @Lstor: I had a feeling I didn't do enough in my default. Could you please tell me how it should be done inside a switch statement? I can't seem to find a firm answer anywhere.
• Lstor: I would've just default: throw std::logic_error("unsupported operator"); or something along those lines.
• Jamal: @Lstor: Ah. I've never seen that one before. I'm still getting an unhandled exception error, though. Could that be compiler-specific?
• Lstor: A warning, surely? A program is not required to catch exceptions, but if an exception leaves main, std::terminate() will be called, which by default will abort() the program. The cleanest approach is to catch the exception somewhere as well.

Answer 2

You are duplicating a lot of code. You might want to put

cout << "\nYour answer is: " << solution << "\nPress Enter to exit.\n";
 cin.get();
 return 0;

after all if conditions, so you will save a lot of space, since these three lines are repeated for each 'if'. In that case, however, you might want to replace your "if"s with "else if". You can also output more than 1 line at a time like this:

cout << "some text" << "some more text" << "and some more";

To end the line, you can also use 'endl' instead of "\n":

cout << "some text" << endl;

So your code can look like this:

 #include <iostream>
 using namespace std;
 int main()
 {
 //it is always a good idea to initialize variables right away. 
 char operation = 0;
 double first_number = 0;
 double second_number = 0;
 double solution = 0;
 cout << "This is a basic calculator program, enter the first number." << endl;
 cout << "First number:";
 cin >> first_number;
 cout << endl << "Enter second number:";
 cin >> second_number;
 cout << endl << "Enter an operation to perform, choose one from this list: +,-,/,*\n";
 cout << "Enter your operation:";
 cin >> operation;
 cin.ignore();
 if (operation != '+' && operation != '-'&& operation != '/' && operation != '*')
 {
 cout << endl << "Invalid operation! Aborting!" << endl << "Press Enter to exit.";
 cin.get();
 return 1;
 }//end if
 if (operation == '+')
 solution = first_number + second_number;
 else if (operation == '-')
 solution = first_number - second_number;
 else if (operation == '*')
 solution = first_number * second_number;
 else if (operation == '/')
 {
 if ( second_number == 0 )
 {
 cout << endl << "You can't divide by zero! Aborting!";
 cout << endl << "Press Enter to exit.";
 cin.get();
 return 2;
 }//end if
 solution = first_number / second_number;
 }//end if
 ///Output starts here
 cout << endl << "Your answer is: " << solution << endl << "Press Enter to exit." << endl;
 cin.get();
 return 0;
 }//end main

Answer 3

You might want to check that your two numbers coming in are in fact numbers, if someone were to enter a letter, your program will crash. You might also want to split the operations up into other functions, it makes code more readable/ debug-able. Lastly, when using cout and cin, don't create new lines using the backslash n character \n, use << endl; which simply ends the line.

• juanchopanza: Actually std::endl does more than just ending the line. By default it flushes the output buffer, so it may be better to avoid overusing it. See here.