1
\$\begingroup\$

The "nasty implementation" I mentioned is within the void calculator() inside of calculator.cpp. Everything goes well as long as user don't type in the erroneous syntax. But I think this isn't the good idea to implement it this way. Can I do it the other way around with more reasonable implementation without losing the functionality of being able to calculate infinitely(not really,I know).

main.cpp:

#include <iostream>
#include "calculator.h"

int main()
{
    calculator();
    return 0;
}

calculator.cpp:

#include <iostream>
#include <cassert>
#include "calculator.h"

int getIntInput()
{
    int x{};
    std::cin >> x;
    assert("Syntax Error!!" && !std::cin.fail());
    return x;
}

char getOperatorInput()
{
    char x{};
    std::cin >> x;
    bool b{x == '+' || x == '-' || x == '*' || x == '/' || x == '^'};
    assert("Syntax Error!!" && b);
    return x;
}

int calculate(int x)
{
    char op{getOperatorInput()};
    int tmp{getIntInput()};
    if(op == '+')
    {
        return x + tmp;
    }
    else if(op == '-')
    {
        return x - tmp;
    }
    else if(op == '*')
    {
        return x * tmp;
    }
    else if(op == '/')
    {
        return x / tmp;
    }
    return 0;
}

void calculator()
{
    std::cout << "Enter your expression : ";
    int64_t result{};
    int loopCount{0};
    do{
            if(loopCount == 0)
                result = getIntInput();
            result = calculate(result);
            loopCount++;
    }
    while(std::cin.peek() != '\n');
    std::cout << "Your result is : " << result << '\n';
}

calculator.h:

#ifndef CALCULATOR_H
#define CALCULATOR_H

int getIntInput();
char getOperatorInput();
void calculator();

#endif // CALCULATOR_H
```
\$\endgroup\$
1
\$\begingroup\$

Some general comments:

  • I think your program is small enough not to warrant any functions. Of course, this can be a matter of taste. It's also reasonable to do input parsing, for instance, in a separate function.

  • To avoid hardcoding the possible binary operators, I suggest taking a more data-driven approach and storing all possibilities in a suitable container (like a map).

  • You use integers as your operands, but you support division. Because you truncate the result into an integer type, 1/2 will equal 0 in your calculator. For this reason, you might want to use a floating point type.

We could rewrite your program into the following 50 lines or so:

#include <iostream>
#include <limits>
#include <map>
#include <functional>
#include <cassert>

int main()
{
    typedef std::function<float(float, float)> binary_op;
    const std::map<char, binary_op> binary_ops =
    {
        { '+', std::plus<float>() },
        { '-', std::minus<float>() },
        { '*', std::multiplies<float>() },
        { '/', std::divides<float>() },
        // NOTE: see how easy adding new functionality is.
        { '#', [](float x, float y) { return x * x * x + y; } }
    };

    std::cout << "Enter your expression: ";
    float result = 0.0;

    float x;
    float y;
    char op;

    if (!(std::cin >> x >> op >> y))
    {
        std::cout << "ERROR!\n";
        return 0;
    }

    const auto op_iter = binary_ops.find(op);
    // Handle errors as you wish.
    assert(op_iter != binary_ops.end());
    result = op_iter->second(x, y);

    while (std::cin.peek() != '\n')
    {
        if (!(std::cin >> op >> y))
        {
            std::cout << "ERROR!\n";
            return 0;
        }

        const auto op_iter = binary_ops.find(op);
        // Handle errors as you wish.
        assert(op_iter != binary_ops.end());

        result = op_iter->second(result, y);
    }

    std::cout << "Result: " << result << "\n";
}

Comments about this solution:

  • We do not correctly handle operator precedence. If you want to do this, I suggest looking at something like the Reverse Polish notation.

  • The std::map now implements the data-driven approach as suggested: you easily add more binary operators as long as they satisfy the required interface. That is, any operator will do as long as it takes two operands (castable to floats) and returns a float. For example, you could add in a fifth line for operator '#' (or whatever you want to call it), and parsing etc. is done automagically for you.

\$\endgroup\$
  • \$\begingroup\$ I'm a learner sir. Thanks for your fix. I askd you to fix it so I can learn it. I know about the precedence problem but I'm thinking of taking in the input as a stream with getline() and access it character by character to determine the sequence but I think that could be computationally expensive. How can I fix that? \$\endgroup\$ – Michael D. Blake May 20 at 5:07
  • \$\begingroup\$ @MichaelDBlake See my edit and particularly the Reverse Polish notation. \$\endgroup\$ – Juho May 20 at 6:07

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.