HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Four-function calculator design

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewcppstackoverflowc++beginnercalculator
designfourfunctioncalculator

Problem

I am new to C++ and am looking to expand my knowledge. Below is a simple program that I've made. I would like to know how it could be improved, in any way. The introduction of new ways to do things is what I am looking for. These could be anything from improving efficiency to validating input - just whatever you think is most important or beneficial.

float calculate(float x, char y, float z) {
    float answer;

    switch (y) {

        case '+':
            answer = x + z;
            break;

        case '-':
            answer = x - z;
            break;

        case '/':
            answer = x / z;
            break;

        case '*':
            answer = x * z;
            break;

        default:
            return(0);
    }

    cout > "; 
    cin >> num1 >> aOp >> num2;
    cout << calculate(num1, aOp, num2) << endl << endl;
}

Solution

-
Headers. Did you leave out the includes and function prototype at the top for simplicity or something? If not, you need to put them here otherwise your program will not compile.

-
Function-returning and displaying. The last statement in your function along with the printing of the function's return value in main() will work, but it's not a good way to write it.

Instead, move the cout

-
Function arguments/parameters. You may want to keep like-types together for clarity and so that you don't mismatch them, which would cause bugs. Whichever order is easiest to remember.

For instance, consider these:

calculate(num1, num2, aOp);            // function call
calculate(float x, float y, char z) {} // function definition


-
Early return in switch. Instead of having a local variable to update and return at the end, you could instead return from the respective case:

switch (y) {
    case '+': return x + z;
    case '-': return x - z;
    case '/': return x / z;
    case '*': return x * z;
    default: std::logic_error("invalid operator"); // from 
}


Notice this default statement. If the user enters an invalid operator, but no input validation was done beforehand, and exception will be thrown. Overall, it's best to have a useful default, and this is one example.

-
You should also make sure the user isn't dividing by 0. If so, don't let the calculation take place, otherwise there will be problem. For such a case, you can just terminate from main()` early:

if (num2 == 0 && aOp == '/')
{
    std::cout << "You cannot divide by 0!  Terminating...";
    return EXIT_FAILURE;
}

Code Snippets

calculate(num1, num2, aOp);            // function call
calculate(float x, float y, char z) {} // function definition
switch (y) {
    case '+': return x + z;
    case '-': return x - z;
    case '/': return x / z;
    case '*': return x * z;
    default: std::logic_error("invalid operator"); // from <stdexcept>
}
if (num2 == 0 && aOp == '/')
{
    std::cout << "You cannot divide by 0!  Terminating...";
    return EXIT_FAILURE;
}

Context

StackExchange Code Review Q#24663, answer score: 6

Revisions (0)

No revisions yet.