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

Jul 5, 2012 at 2:52am
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 Jul 5, 2012 at 2:54am
Jul 5, 2012 at 3:07am
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 Jul 5, 2012 at 3:08am
Jul 5, 2012 at 3:11am
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?
Jul 5, 2012 at 3:11am
thanks, yah I wasn't to sure about where the statements should go in relation with the function
Jul 5, 2012 at 3:23am
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;
	}
Jul 5, 2012 at 3:28am
ooo very nice.
Jul 5, 2012 at 5:42am
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 Jul 5, 2012 at 7:13am
Topic archived. No new replies allowed.