5
\$\begingroup\$

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 <<"= "; return answer;
}

int main() {
    float num1;
    float num2;
    char aOp;

    cout << ">> "; 
    cin >> num1 >> aOp >> num2;
    cout << calculate(num1, aOp, num2) << endl << endl;
}
\$\endgroup\$
5
\$\begingroup\$
  • 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 <<"= "; from your function (just keep return answer;) to your std::cout statement in main().

  • 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 <stdexcept>
    }
    

    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;
    }
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.