take a look at my calculator? have any suggestions for improvement?

I just started learning c++ once again after a few months of break.. during that break though I learned all the basics of css and html. Do you have any suggestions for this smiple arithmetic calculator? any would help! :) I basically used most of the knowledge I had about c++.. I kind of forgot how to make proper classes and objects but I probably would have written this differently if I did remember.


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
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
#include <iostream>
#include <windows.h>
using namespace std;

int main()
{
    int x,y,principle, add,sub,multi,ball,div;
    ball = 0;
    system("color a");


    cout<<"Thankyou for using this simple arithmetic calculator! - ocunder"<<endl;
    cin.get();
    cout<<"input number: ";
    cin>>x;
    cout<<"input number: ";
    cin>>y; cout<< endl;
    cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;
    div = 4;





    while (ball == 0)
{
    cin>> principle;

    if (principle == add)
    {
    x+y;
    cout<<endl<<"answer: "<<x+y<<endl<<endl;

    system("PAUSE");
    system("cls");



    cout<<"input number: ";
    cin>>x;
    cout<<"input number: ";
    cin>>y; cout<< endl;
    cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;


    }

    if (principle == sub)
    {
    x-y;
   cout<<endl<<"answer: "<<x-y<<endl<<endl;
    system("PAUSE");
    system("cls");

    cout<<"input number: ";
    cin>>x;
    cout<<"input number: ";
    cin>>y; cout<< endl;
    cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;

    }

    if (principle == multi)
    {
    x*y;
   cout<<endl<<"answer: "<<x*y<<endl<<endl;
    system("PAUSE");
    system("cls");

    cout<<"input number: ";
    cin>>x;
    cout<<"input number: ";
    cin>>y; cout<< endl;
    cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;

    }

        if (principle == div)
    {
    x/y;
    cout<<endl<<"answer: "<<x/y<<endl<<endl;
    system("PAUSE");
    system("cls");

    cout<<"input number: ";
    cin>>x;
    cout<<"input number: ";
    cin>>y; cout<< endl;
    cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;

    }


}




    return 0;
}
Last edited on
Space your code out a bit. Here's a few examples:

1
2
3
4
5
6
7
cin>>x;
// verses
cin >> x;

if(x+y>0&&x+y<10)
// verses
if (x + y > 0 && x + y < 10)


Also, a lot of your code isn't indented properly. Here's an example from something I'm working 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
void Display_Room_Information (const Area & cArea)
{
	cout << cArea.szCurrentArea << endl;
	for (unsigned int i = 0; i < cArea.szCurrentArea.length(); i++)
		cout << "-";
	cout << endl;

	cout << cArea.szAreaDescription << endl;

	if (cArea.MonsterInRoom == true)
		cout << "There are monsters in the area.\n";

	if (cArea.szNorthArea != "None")
		cout << "You can head North...\n";
	if (cArea.szEastArea != "None")
		cout << "You can head East...\n";

	if (cArea.szSouthArea != "None")
		cout << "You can head South...\n";
	if (cArea.szWestArea != "None")
		cout << "You can head West...\n";

	cout << endl;
}


This looks quite easy to read, now take a look at the difference:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
void Display_Room_Information (const Area & cArea)
{
cout << cArea.szCurrentArea << endl;
for (unsigned int i = 0; i < cArea.szCurrentArea.length(); i++)
cout << "-";
cout << endl;

cout << cArea.szAreaDescription << endl;
if (cArea.MonsterInRoom == true)
cout << "There are monsters in the area.\n";
if (cArea.szNorthArea != "None")
cout << "You can head North...\n";
if (cArea.szEastArea != "None")
cout << "You can head East...\n";
if (cArea.szSouthArea != "None")
cout << "You can head South...\n";
if (cArea.szWestArea != "None")
cout << "You can head West...\n";

cout << endl;
}


I know I'm not talking specifically about the functionality of your program, but it's VERY important to have neat and organized code, so other people can actually read it without straining their brain.
Last edited on
1
2
3
4
5
6
7
int x,y,principle, add,sub,multi,ball,div;
...
cout<<"choose your principle  add = 1, sub = 2, multi = 3, div = 4 : ";
    add = 1;
    sub = 2;
    multi = 3;
    div = 4;


What is this? What is your logic behind having a variable for each operand? Why not just use enum?
thanks, yah I wasn't to sure about where the statements should go in relation with the function
Try this on for size

Enter "0" to terminate or "433*16" or " 433 * 16". No error checking so it must be number function number

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
#include <iostream>

using namespace std;

int main() {
	int	Num_A, Num_B, Answer;
	char Function;
	
	cout << "Thank-you for using this simple arithmetic calculator! - ocunder" << endl;
	
	do	{
		cout << " Equation: ";
		cin >> Num_A;
		
		if (Num_A) {
			cin >> Function >> Num_B;
			
			switch ( Function ) {
				case	'+':
					Answer = Num_A + Num_B;
					break;
				case	'-':
					Answer = Num_A - Num_B;
					break;
				case	'*':
				case	'x':
				case	'X':
					Answer = Num_A * Num_B;
					break;
					
				case	'/':
					Answer = Num_A / Num_B;
					break;
				
				default:
					cout << "HUGH!!!" << endl;
					Answer = 0;
					continue;
				}
			
			cout << "\t= " << Answer << endl;
			}
		else
			break;
			
		} while (1);
		
    return 0;
	}
ooo very nice.
case	'/':
	Answer = Num_A / Num_B;
	break;

This might result in break if Num_B =0;

so check for the condition,

case	'/':
        if(Num_B == 0)
        {
             cout<<"Divide by zero error";
         }
	Answer = Num_A / Num_B;
	break;
Last edited on
Topic archived. No new replies allowed.