10
\$\begingroup\$

This is "my" version of a basic C++ calculator, that requires a single line input (like 1+1) to calculate the result. I have been (self)learning the language for 2 months now. I don't pretend to have invented this code, but I haven't taken the time to search for this variation.

The functions included are:

  • Addition: 1 + 1 = 2

    Input in program: 1+1

  • Subtraction: 1 - 1 = 0

    Input in program: 1-1

  • Multiplication: 1 * 1 = 1

    Input in program: 1*1

  • Division: 1 / 1 = 1

    Input in program: 1/1

  • Exponentiation: 2 ^ 2 = 4

    Input in program: 2^2

#include <iostream>  
#include <string>

using namespace std;

class Math_Functions
{
private:
    double result;

public:
    double addition(double, double);
    double subtraction(double, double);
    double multiplication(double, double);
    double division(double, double);
    double exponentiation(double, double);
} math;

double Math_Functions::addition(double a, double b)
{
    result = a + b;
    return result;
}

double Math_Functions::subtraction(double a, double b)
{
    result = a - b;
    return result;
}

double Math_Functions::multiplication(double a, double b)
{
    result = a * b;
    return result;
}

double Math_Functions::division(double a, double b)
{
    result = a / b;
    return result;
}

double Math_Functions::exponentiation(double a, double b)
{
    result = 1;
    for (int i = 0; i < b; i++)
    {
        result *= a;
    }
    return result;
}

int main()
{
    double a, b;
    string function;
    bool loop = 1;

    while(loop)
    {
        system("CLS");
        cin >> a;
        function = getchar();
        cin >> b;

        switch(function[0])
        {
            case '+' : cout << math.addition(a, b); break;
            case '-': cout << math.subtraction(a, b); break;
            case '*': cout << math.multiplication(a, b); break;
            case '/': cout << math.division(a, b); break;
            case '^': cout << math.exponentiation(a, b); break;
        }
        cout << endl << "Press Any Key to Continue . . . ";
        cin.sync();
        cin.ignore();
    }
    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Look up the concept of the Command Pattern. It will remove the need for a switch. \$\endgroup\$ – Martin York Jan 19 '15 at 22:51
10
\$\begingroup\$

If you're about to perform division, you must first check for 0 as the given divisor. To prevent this, it may be best to warn the user, and make sure they input a valid number before proceeding.

For the exponentiation, you can have a "shortcut" by returning 1 instantly if b is 0. There's no need to attempt calculations since anything raised to the 0 power is always 1. You should also handle negative powers in some way. The code will break if the user inputs such a value for b. This loop cannot handle all of these possible values.

\$\endgroup\$
  • \$\begingroup\$ Now as I read it it seems so logical it makes me wonder where my mind was when I was writing the code down... Rewriting in process. \$\endgroup\$ – skorpi7478 Jan 19 '15 at 21:36
  • \$\begingroup\$ Division by zero in the context of floating point operation is well defined in C++, the possible results are either + or -infinity or NaN (not a number) \$\endgroup\$ – ChrisWue Jan 20 '15 at 7:32
9
\$\begingroup\$

Avoid using namespace std

It's considered bad practice to do using namespace std.

Don't use a string when a simple char is enough

Since function must be a single character, there's no need to use a string for it and then use function[0] in the switch. You could use a simple char:

char function = getchar();

Use true and false for booleans

Instead of:

bool loop = 1;

You should write:

bool loop = true;

However, since the while loop in the main function never terminates, the boolean variable is pointless, and you could just write:

while (true) {
    // ...
}

Why is result a member variable?

I don't see why result is a private variable of the class. Why not make it a local variable instead? In fact, most methods don't need a local variable at all and could return the output of their calculation directly, for example:

double Math_Functions::addition(double a, double b)
{
    return a + b;
}

double Math_Functions::subtraction(double a, double b)
{
    return a - b;
}
\$\endgroup\$
  • \$\begingroup\$ Point taken for everything! Thank you very much. Guess I'll have to rewrite all my previous programs with these enlightments. \$\endgroup\$ – skorpi7478 Jan 19 '15 at 21:34
7
\$\begingroup\$

I am going to suggest couple of changes to your code.

Providing a clean way to terminate the program

Your code does not provide a clean way to terminate the program. Once started, you have to terminate the program by pressing the equivalent of Ctrl+C, killing the process form the Task Manager, or some such indirect mechanism.

There are lots of ways to terminate a program cleanly. One of the easiest things to do is look for q or Q as the first character of the input to exit the program.

However, if you allow for such a method, you'll need to change how you receive user input and process them.

Instead of

    cin >> a;
    function = getchar();
    cin >> b;

you'll need to use:

    std::string line;
    std::getline(cin, line);

    // Ignore blank lines.
    if (line.empty() )
    {
        continue;
    }

    // Check for `q` or `Q`. If found, break out of the loop.
    if ( line[0] == `q'` or line[0] == `Q`)
    {
        break;
    }

    // Create a stringstream and extract the data from the line.
    std::istringstream inStream(line);
    inStream >> a;
    inStream >> function;
    inStream >> b;

Dealing with bad input

If the user provides bad input, whether on purpose or on accident, you are not dealing with it at all. When extracting the a, function, and b, check whether the input stream is still in valid state. If not, print a message and go to the start of the while loop.

    inStream >> a;
    inStream >> function;
    inStream >> b;

    if ( !inStream )
    {
       std::cerr << "'" << line << "' contains invalid input.\n"
       continue;
    }
\$\endgroup\$
1
\$\begingroup\$

For exponentiation, you define b as double. In that case, your code is not in general correct, as you are using a simple integer loop to perform exponentiation. It is only valid for nonnegative integer exponents. It could also take a really long time if b is a large number. Use

return pow(a,b);

to allow arbitrary real exponents. The function is in <cmath>.

If you really wanted to implement just positive integer exponents, you also didn't use the most sensible solution. Your loop performs b multiplications, which can be reduced to log2(b) multiplications (basically the number of nonzero bits in b) if you use exponentiation by squaring.

http://en.wikipedia.org/wiki/Exponentiation_by_squaring

\$\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.