class based claculator confusion

Now i'm no pro at C++ and am pretty new and so I kinda ran into a wall with this design for a class based calculator. The idea was that I make a class that contains all of the different types of math operations such as addition, subtraction, multiplication, and division. These as you can see near the start of the code are all under the class MathOps and have a public function to grab them from the private function (Which I think I messed up) and they are then called to make an object later in the code. Overall I need some help figuring out how to properly call the operation variables in the class for use in the actual math later on.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
 #include <iostream>
#include <cmath>
using namespace std;
// Advanced Calculator Project

//Naming Operations
class MathOps {
public:
	void setAddition(int x, int y) {
		Addition = x + y;
	}
	void setSubtraction(int x, int y) {
		int answer = x - y;
	}
	void setMultiplication(int x, int y) {
		int answer = x / y;
	}
	void setDivison(int x, int y) {
		int answer = x * y;
	}
	int getAddition() {
		return Addition;
	}
	int getSubtraction() {
		return Subtraction;
	}
	int getMultiplication() {
		return Multiplication;
	}
	int getDivison() {
		return Divison;
	}
private:
	int Addition;
	int Subtraction;
	int Multiplication;
	int Divison;
};

//Calculator Begins
int main()
{
//Function Specific Variables(FSV)
	int MenuChoice;
	int answer;
	int x;
	int y;

	//Main Menu
	cout << "Please select a operation\n" << endl;
	cout << " 1. Addition" << endl;
	cout << " 2. Subtraction" << endl;
	cout << " 3. Multiplication" << endl;
	cout << " 4. Divison\n" << endl;
	cout << "What is your choice (select by inserting its number)? " << endl;
	cin >> MenuChoice;

	//Operations
	if (MenuChoice == 1) {
		MathOps MathObject;
		MathObject.setAddition(x, y);
		cout << "Insert your first number\n" << endl;
			cin >> x;
			cout << "Insert your second number\n" << endl;
			cin >> y;
			cout << "your answer is " << answer << endl;
	}
	else if (MenuChoice == 2) {
		MathOps MathObject;
		MathObject.setSubtraction(x, y);
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		cout << "your answer is " << answer << endl;
	}
	else if (MenuChoice == 3) {
		MathOps MathObject;
		MathObject.setMultiplication(x, y);
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		cout << "your answer is " << answer << endl;
	}
	else if (MenuChoice == 4) {
		MathOps MathObject;
		MathObject.setDivison(x, y);
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		cout << "your answer is " << answer << endl;
	}
	return 0;
Last edited on
Not a bad use of classes here.

I'll make a couple main points first:
Remember Scope. Your object is only visible in the scope in which it was defined. If you create it inside an if statement, it'll go out of scope (and be destroyed) when you leave that if statement.
You probably want this object to stay the whole time the user is using the program, right? You can declare it in the same place you declared "MenuChoice". Doing that will allow the same instance to stay in scope for the whole time.

There are some instances where you want to create an object and have it go out of scope. If you do that, just know that you cannot reference that instance outside of that scope.

I also notice you call MathObject.set... before you ask the user for input. When you call MathObject.set... before you get the user input, you won't get the right values. You'll be sending the old values in, when you really want to send the values that the user just entered.

You can move MathObject.set... to the line right after cin >> y; and you'll calculate it using the new values.

I would move MathOps MathObject; to the same area that you declare the MenuChoice, x, y, and answer. That way you are doing all these operations with the same instance. Inside your if statements, a new object is created every time and it is destroyed right when execution leaves the if statement.

I'm glad to see the usage of get/set methods. Many, many APIs allow the user to interact only using get/methods. It prevents a user from doing something with your class that they shouldn't be doing. This is not a bad habit to get into (using setters and getters, that is).
Does this look better as an example? I moved the primary object to the start of the main function and put the MathOps.set after the last cin << in the if statement for the sake of having updated results. Just for clarification does this mean that I can remove Mathops Mathobject; from all if statements and else if statements in the code since it is called off the bat?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
//Function Specific Variables(FSV)
	MathOps MathObject;
	int MenuChoice;
	int answer;
	int x;
	int y;

	//Main Menu
	cout << "Please select a operation\n" << endl;
	cout << " 1. Addition" << endl;
	cout << " 2. Subtraction" << endl;
	cout << " 3. Multiplication" << endl;
	cout << " 4. Divison\n" << endl;
	cout << "What is your choice (select by inserting its number)? " << endl;
	cin >> MenuChoice;

	//Operations
	if (MenuChoice == 1) {
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		MathObject.setAddition(x, y);
		cout << "your answer is " << answer << endl;
	}
Last edited on
Yes, you can remove that line from all of the if statements. You're instantiating right off the bat and you can use the same one the entire time.

Actually, one thing. You want to print the result of the addition, right? That is stored in "Addition", a private variable. If you want to show it to the user, you'll have to retrieve it first.
You can do answer = MathObject.get...; or cout << "your answer is " << MathObject.get...;
For the second one, you aren't storing it in answer, your just printing the return value of the call to get..., which is an integer, which can be printed. The first one stores the result in "answer" and you can print that. But answer will only be updated when you do it yourself. It won't update when MathObject changes "Addition".
So I did a little revision and this is how my code ended up. It seems to almost be where I want it but the issue is that I still have to add the answer = x + y; in each case of my switch when the idea is to have the class govern how the math is done and as such use the class to get the final answer.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
#include <iostream>
#include <cmath>
using namespace std;
//Advanced Calculator Project

//Naming Operations
class MathOps {
public:
	void setAddition(int x, int y) {
		Addition = x + y;
	}
	void setSubtraction(int x, int y) {
		Subtraction = x - y;
	}
	void setMultiplication(int x, int y) {
		Multiplication = x / y;
	}
	void setDivison(int x, int y) {
		Divison = x * y;
	}
	int getAddition() {
		return Addition;
	}
	int getSubtraction() {
		return Subtraction;
	}
	int getMultiplication() {
		return Multiplication;
	}
	int getDivison() {
		return Divison;
	}
private:
	int Addition;
	int Subtraction;
	int Multiplication;
	int Divison;
};

int main()
{
//Function Specific Variables(FSV)
	MathOps MathObject;
	int MenuChoice;
	int answer;
	int x;
	int y;

	//Main Menu
	cout << "Please select a operation\n" << endl;
	cout << " 1. Addition" << endl;
	cout << " 2. Subtraction" << endl;
	cout << " 3. Multiplication" << endl;
	cout << " 4. Divison\n" << endl;
	cout << "What is your choice (select by inserting its number)? " << endl;
	cin >> MenuChoice;

	//Operations
	switch (MenuChoice) {
	case 1:
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		MathObject.setAddition(x, y);
		answer = x + y;
		cout << "your answer is " << answer << endl;
		break;
	case 2:
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		MathObject.setSubtraction(x, y);
		answer = x - y;
		cout << "your answer is " << answer << endl;
		break;
	case 3:
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		MathObject.setMultiplication(x, y);
		answer = x * y;
		cout << "your answer is " << answer << endl;
		break;
	case 4:
		cout << "Insert your first number\n" << endl;
		cin >> x;
		cout << "Insert your second number\n" << endl;
		cin >> y;
		MathObject.setDivison(x, y);
		answer = x / y;
		cout << "your answer is " << answer << endl;
		break;
	default:
		cout << "That is not a valid option" << endl;
	}
	return 0;
}
Last edited on
At a glance, I only notice one issue. You don't update "answer". It'll just print some old value or garbage value from memory. What you'll want to do is update "answer" by doing answer = MathObject.getAddition(); (or whichever get... you need to do for that if-statement) before you print it so that it is updated before you print it to the screen. Now your MathObject will be constructed when main starts and will you'll be using the same instance throughout your entire program.

Your code is very easy to read. I like that. Please continue to make code that is easy to read. You have no idea how much time we can spend just making someone's code readable so that we can debug it. Having pretty code makes us want to look at much more.
As you start making more complex programs and your classes become more complex, document your code as you go along. It'll make it easier for you to look at later and much easier for someone else who didn't write it to understand how it works.
Topic archived. No new replies allowed.