7
\$\begingroup\$

I am new to object-oriented concepts. The following is my attempt at creating a basic calculator using class and operator overloading concepts. Please review it for improvements. Also, how do I make it more intuitive? I want to display: +,-,*,/ as options and use switch on them.

#include <iostream>
#include <conio.h>

using namespace std;

class calculator
{
    private:
        float val;

    public:
        calculator(): val(0)
        {}

        void getdata()
        {           
            cout << "enter number: ";
            cin >> val;
        }

        void showdata()
        {
            cout << "value: " << val << endl;
        }

        calculator operator + (calculator) const;
        calculator operator - (calculator) const;
        calculator operator * (calculator) const;
        calculator operator / (calculator) const;
};

calculator calculator::operator + (calculator arg2) const
{
    calculator temp;
    temp.val = val + arg2.val;
    return temp;
}

calculator calculator::operator - (calculator arg2) const
{
    calculator temp;
    temp.val = val - arg2.val;
    return temp;
}

calculator calculator::operator * (calculator arg2) const
{
    calculator temp;
    temp.val = val * arg2.val;
    return temp;
}

calculator calculator::operator / (calculator arg2) const
{
    calculator temp;
    temp.val = val / arg2.val;
    return temp;
}

void main()
{
    calculator obj1, obj2, obj3;
    char ch;
    int choice;

    obj1.getdata();
    cout << "1st value entered: ";
    obj1.showdata();
    cout << endl;

    obj2.getdata();
    cout << "2nd value entered: ";
    obj2.showdata();
    cout << endl;

    cout << "Input choice as integer: 1: +,   2: -,   3: *,   4:/     ";
    cin >> choice;
    cout << endl;

    switch (choice)
    {
        case 1:
            obj3 = obj1 + obj2;
            break;
        case 2:
            obj3 = obj1 - obj2;
            break;
        case 3:
            obj3 = obj1 * obj2;
            break;
        case 4:
            obj3 = obj1 / obj2;
            break;
        default:
            cout << "Invalid choice! " << endl;
    }

    cout << "Result ";
    obj3.showdata();
    cout << endl;

}
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

You can also overload operator>> instead of using getdata():

std::istream& operator>>(std::istream& in, calculator& obj)
{
    return in >> obj.val;
}

Similar approach with showdata(), using operator<<:

std::ostream& operator<<(std::ostream& out, calculator const& obj)
{
    return out << "value: " << obj.val;
}

As for your existing overloads, you shouldn't need to return a calculator. You appear to treat each calculator object as a single value. This makes each one to appear to be single-use, and the more values you wish to enter, the more objects thus the more code you'll need to have.

What you should do is maintain one object and have val maintained as you enter data. However, this will also mean that your arithmetic operators aren't needed. If you wish to keep them, then you'll have to approach this differently. For instance, you can calculate (not merely input) a final value for different calculators and use those operators to get a new value. This may look like a needless approach for a simple calculator, but at least you're still able to utilize these arithmetic operators.

Some miscellaneous notes:

\$\endgroup\$
2
  • \$\begingroup\$ thanks for the suggestions and additional operator overloading tips. That was very helpful. Could you please look into the switch case problem that I mentioned? I need it to be more intuitive. \$\endgroup\$
    – learner
    Feb 1, 2015 at 14:15
  • \$\begingroup\$ @learner: For a switch, it already looks pretty intuitive. The main problem with it now is that it doesn't really stop the user from inputting an invalid choice (the result will be displayed anyway). You may need a loop for this. \$\endgroup\$
    – Jamal
    Feb 1, 2015 at 18:04
0
\$\begingroup\$

Quick review:

#include <conio.h>

File not found. But it seems that this include isn't required, so you can make your code portable by removing this line.

using namespace std;

Don't do that - use the namespace properly.

        float val;

We normally use double unless there's a compelling reason to use a larger or smaller floating-point type.

void main()

The main function must return int.

    char ch;

This variable is never used.

            cin >> val;

This needs some validation that the input could be parsed as a number. If the >> fails, we have uninitialised value in val, making all subsequent calculation meaningless.

            cout << "Invalid choice! " << endl;

Errors should go to std::cerr. There's no need to flush the output stream, so we can use a plain newline (\n) instead of std::endl.


Design review

This looks like you have just learnt about object-oriented design, and are trying to apply it as an exercise. That's fine, but it's not really a natural fit here.

The naming is a bit weird, and doesn't match with real-world expectations. When I'm using a calculator, I don't get another calculator when I perform an addition - I have the same calculator, but with an update number stored in it. You might find that the assignment operators (+= and friends) are a closer match to a real-world calculator.

\$\endgroup\$

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