C++

Forum

 
How to make this better (HW)

ebba (12)
I'm working on this assignment. Part one was to create a program that could do basic calculations with simple fractions (+,-,*,/). Part 2 is to create a program that will find the lowest GCD of 2 numbers. The last part is to combine both of these programs that will do calculations on fractions and return the answer in simplified form. It's making my brain hurt. If anyone's got some pointers, you're the best, please help. 


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
 
//calculator program
#include <iostream>
using namespace std; 

int main (){
	
	char op, slash, repeat; 
	int num1, den1, num2, den2; 

do{	
	cout << "Enter the first fraction: ";
	cin >> num1 >> slash >> den1;
		while (den1 == 0){
			cout << "Denominators cannot be equal to zero. Please reenter entire fraction with valid input: ";
			cin >> num1 >> slash >> den1;
		};
	cout << "Enter operation: ";
	cin >> op;
	cout << "Enter the second fraction: ";
	cin>> num2 >> slash >> den2;
		while (den2 == 0){
			cout << "Denominators cannot be equal to zero. Please reenter entire fraction with valid input: ";
			cin>> num2 >> slash >> den2;
		};
		
	switch (op){	
		case '+':
			cout << "Sum: " << (num1*den2 + num2*den1) << "/" << (den1*den2);
			break;
		case '-': 
			cout << "Difference: " << (num1*den2 - num2*den1) << "/" << (den1*den2); 
			break;
		case '/':
			cout << "Quotient: " << (num1*den2) << "/" << (num2*den1);
			break;
		case '*':
			cout << "Product: " << (num1*num2) << "/" << (den1*den2); 	
			break;
			
		default: break;
	}
	
	do{
		cout << "\nContinue? (y/n): ";
		cin >> repeat; 
	} while (repeat != 'y' && repeat != 'Y' && repeat != 'n' && repeat != 'N');

} while (repeat == 'y' || repeat == 'Y');
	
	return 0;
}


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
 
//gcd program
#include <iostream>
using namespace std;

double gcd (int, int);

int main(){
	
	int num1, num2, small, big, ans;
	
	cout << "Enter first number: ";
	cin >> num1;
	cout << "Enter second number: ";
	cin >> num2;
	
	if (num1 > num2){
		big = num1;
		small = num2;
	}
	else {
		small = num1;
		big = num2;
	}
	
	ans = gcd (small, big);
	
	cout << ans;
	
	return 0;
	
}

double gcd (int small, int big){
	
	int rem;
	
	rem = big%small;
	
	while (rem != 0){
		small = rem;
		rem = big%rem;
	}

	return (small);

}

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
 
//WIP trying to combine them
#include <iostream>
using namespace std; 

double gcd (int, int);

int main (){
	
	char op, slash, repeat; 
	int num1, den1, num2, den2, resNum1, resNum2, big, small, ans; 

do{	
	cout << "Enter the first fraction: ";
	cin >> num1 >> slash >> den1;
		while (den1 == 0){
			cout << "Denominators cannot be equal to zero. Please reenter entire fraction with valid input: ";
			cin >> num1 >> slash >> den1;
		};
	cout << "Enter operation: ";
	cin >> op;
	cout << "Enter the second fraction: ";
	cin>> num2 >> slash >> den2;
		while (den2 == 0){
			cout << "Denominators cannot be equal to zero. Please reenter entire fraction with valid input: ";
			cin>> num2 >> slash >> den2;
		};
		
	switch (op){	
		case '+':
			resNum1 = (num1*den2 + num2*den1);
			resNum2 = (den1*den2);
				if (resNum1 > resNum2){
				big = num1;
				small = num2;
				}
				else {
				small = resNum1;
				big = resNum2;
				}
			ans = gcd (small, big); 
			cout << resNum1/ans << "/" << resNum2/ans;				
			break;
		case '-': 
			resNum1 = (num1*den2 - num2*den1);
			resNum2 = (den1*den2); 
				if (resNum1 > resNum2){
				big = num1;
				small = num2;
				}
				else {
				small = resNum1;
				big = resNum2;
				}
			ans = gcd (small, big); 
			cout << resNum1/ans << "/" << resNum2/ans;				
			break;
		case '/':
			resNum1 = (num1*den2);
			resNum2 = (num2*den1);
				if (resNum1 > resNum2){
				big = num1;
				small = num2;
				}
				else {
				small = resNum1;
				big = resNum2;
				}
			ans = gcd (small, big); 
			cout << resNum1/ans << "/" << resNum2/ans;				
			break;
		case '*':
			resNum1 = (num1*num2);
			resNum2 = (den1*den2);
				if (resNum1 > resNum2){
				big = num1;
				small = num2;
				}
				else {
				small = resNum1;
				big = resNum2;
				}
			ans = gcd (small, big); 
			cout << resNum1/ans << "/" << resNum2/ans;	
			break;
			
		default: break;
	}	
	
	do{
		cout << "\nContinue? (y/n): ";
		cin >> repeat; 
	} while (repeat != 'y' && repeat != 'Y' && repeat != 'n' && repeat != 'N');

} while (repeat == 'y' || repeat == 'Y');
	
	return 0;
}


double gcd (int small, int big){
	
	int rem;
	
	rem = big%small;
	
	while (rem != 0){
		small = rem;
		rem = big%rem;
	}
	
	return (small);
}
Last edited on
Handy Andy (5051)
Hello ebba,

In your third bit of code that combines the two programs I do not see anything wrong with what you have.

Some suggestions I can make:

In the case statements the if/else could be shortened to:
resNum1 > resNum2 ? ans = gcd(num1, num2) : ans = gcd(resNum1, resNum2);. The ? is similar to the then part if an if/else and the : would be the else part. You do not need to set something equal to a new variable, (small or big) you just need to know what to send with what you already have.

After your switch you have:
1
2
3
4
5
 
do
{
	cout << "\nContinue? (y/n): ";
	cin >> repeat;
} while (repeat != 'y' && repeat != 'Y' && repeat != 'n' && repeat != 'N');


You can shorten the while condition like this:
1
2
3
4
5
6
7
 
	do
	{
		std::cout << "\n\nContinue? (y/n): ";
		std::cin >> repeat;
		repeat = std::toupper(repeat);  // <--- include header file <cctype>.

	} while (repeat != 'Y' && repeat != 'N');

This will also shorten the condition of the outer do/while loop. Which would be written as while (repeat == 'Y');. The above do/while loop is not really necessary unless you want to restrict the answer to only "Y" or "N". The condition in the outer do/while loop is all that is needed.

If you look at my example for the do/while you will notice that the {}s are in the same column as the "d" in "do". There is nothing wrong with what you have done, but this makes it easier to read and match up the {}s.

The last thing I will mention is using namespace std;. This is a bad idea and WILL get you in trouble some day. You can search here to read why this is bad and check out this link:
http://www.lonecpluspluscoder.com/2012/09/22/i-dont-want-to-see-another-using-namespace-xxx-in-a-header-file-ever-again/

Hpe that helps,

Andy
Topic archived. No new replies allowed.