C++

Forum

 
Help factoring a section of a program to a function

IdivideBy0 (27)
Hello all,

I was wondering if I could get some input on a project I turned in for class. The instructor knocked 2 points off of the assignment of a possible 15 total. He says that I should have factored my code that checks for a zero denominator into a function. With the way I output my messages based on the operand selected, I just don't see it? Any help would be greatly appreciated. Thank you.

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
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
 
#include <iostream>
#include <iomanip>
#include <string>

using namespace std;

void addFractions(int,int,int,int);
void subtractFractions(int,int,int,int);
void multiplyFractions(int,int,int,int);
void divideFractions(int,int,int,int);
void menu();

int main()
{	
	menu();
	
return 0;
}

void menu()
{
	bool zero_found = false; // Zero based fractions are NOT allowed.
	bool valid = false;	     // Make sure user entered a valid operation.
	char operation = ' ';    // Holds the value of our math operator.
	char solidus1, solidus2; // We need to throw out the / symbol when inputing data.
	int numerator1, denominator1, numerator2, denominator2;

	string instructions = "Data is entered in the following format: \nnumerator/denominator \n" ;
	
	cout << setw(50) << "Welcome to my fraction calculator program:" << endl << endl;
	cout << "Please select from the following functions:" << endl;
	cout << "'+' To add fractions press + then enter" << endl;
	cout << "'-' To subtract fractions press - then enter" << endl;
	cout << "'*' To multiply fractions press * then enter" << endl;
	cout << "'/' To divide fractions press / then enter" << endl;
	cout << endl << ">>";
	
	do{
	
	cin >> operation;

	if(operation == '+') 
	{
		
		do{
		cout << instructions;
		cout << "Enter first fraction to add :" << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to add:" << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
			if((denominator1 == 0) || (denominator2 == 0))  //tests for zero
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				zero_found = true;
			}
			else
			{
				zero_found = false;
				valid = true; 	
				addFractions(numerator1,denominator1,numerator2,denominator2); // perform calculations
			}

		}
		while(zero_found == true);
			
				
	}
	else if(operation == '-')
	{
		do{
		cout << instructions;
		cout << "Enter first fraction to subtract:" << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to subtract:" << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
		if((denominator1 == 0) || (denominator2 == 0)) 
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				zero_found = true;
			}
			else
			{
				zero_found = false;
				valid = true; 	
				subtractFractions(numerator1,denominator1,numerator2,denominator2);
			}

		}
		while(zero_found == true);
		
	}
	else if(operation == '*')
	{
		do{
		cout << instructions;
		cout << "Enter first fraction to multiply:" << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to multiply:" << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
		if((denominator1 == 0) || (denominator2 == 0)) 
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				zero_found = true;
			}
			else
			{
				zero_found = false;
				valid = true; 	
				multiplyFractions(numerator1,denominator1,numerator2,denominator2);
			}

		}
		while(zero_found == true);
	}
	else if(operation == '/')
	{
		do{
		cout << instructions;
		cout << "Enter first fraction to divide:" << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to divide:" << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
		if((denominator1 == 0) || (denominator2 == 0)) 
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				zero_found = true;
			}
			else
			{
				zero_found = false;
				valid = true; 	
				divideFractions(numerator1,denominator1,numerator2,denominator2);
			}

		}
		while(zero_found == true);
	}
	else 
	{
		cout << "Please select a valid operation!" << endl;
	}

	}
	while(!valid);

}

void addFractions(int numerator1, int denominator1, int numerator2, int denominator2)
{
	int numeratorSum, denominatorSum;

	numeratorSum = numerator1 * denominator2 + numerator2 * denominator1;
	denominatorSum = denominator1 * denominator2;

	cout << "The value of " << numerator1 << "/" << denominator1 << " + " << numerator2 << "/" << denominator2 
		 << " = " << numeratorSum << "/" << denominatorSum <<endl;	
}

void subtractFractions(int numerator1, int denominator1, int numerator2, int denominator2)
{
	int finalNumerator, finalDenominator;

	finalNumerator = numerator1 * denominator2 - numerator2 * denominator1;
    finalDenominator = denominator1 * denominator2;

	cout << "The value of " << numerator1 << "/" << denominator1 << " - " << numerator2 << "/" << denominator2 
		 << " = " << finalNumerator << "/" << finalDenominator << endl;	

}

void multiplyFractions(int numerator1, int denominator1, int numerator2, int denominator2)
{
	int finalNumerator, finalDenominator;

	finalNumerator = numerator1 * numerator2;
    finalDenominator = denominator1 * denominator2;

	cout << "The value of " << numerator1 << "/" << denominator1 << " * " << numerator2 << "/" << denominator2 
		 << " = " << finalNumerator << "/" << finalDenominator << endl;	

}

void divideFractions(int numerator1, int denominator1, int numerator2, int denominator2)
{
	int finalNumerator, finalDenominator;

	finalNumerator = numerator1 * denominator2;
    finalDenominator = denominator1 * numerator2;

	cout << "The value of " << numerator1 << "/" << denominator1 << " / " << numerator2 << "/" << denominator2 
		 << " = " << finalNumerator << "/" << finalDenominator << endl;	

}
Chervil (7320)
Well, a lot of the code in function menu() is very repetitive. At least I would take the parts which are common to all cases out and put them in a separate function.

What I see at first glance is the only difference between the various cases is a couple of words which are output, and which function is subsequently called. So you could use a string for the words, such as "add", "subtract" etc. and pass that as a parameter to the function. When it returns with the validated input, call the required function.

If you do that, function menu() will get shorter and easier to read. And making the code easy to read and follow is a big part of the task.

Now that may not be exactly what your instructor said, there is room for some difference of opinion here.
AbstractionAnon (6954)
I agree with your prof on this one. The following sets of lines do eaxtly the same thing and should have been in a function.
51-57
78-84
104-111
129-135

1
2
3
4
5
6
7
8
9
 
bool check_denominators (int denom1, int denom2)
{  if((denom1 == 0) || (denom2 == 0)) 
    {   cout << endl << "Error: denominator can not be zero!" << endl;
         cin.clear();
         cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
         return false;   // denoms not good
    }
    return true;  // denoms ok
}


Chervil (7320)
This is my suggested function:
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
 
void getData(std::string op, int &num1, int & denom1, int &num2, int &denom2)
{
    string instructions = "Data is entered in the following format: \n"
                          "numerator/denominator \n" ;

    char solidus1, solidus2; // We need to throw out the / symbol when inputing data.

    bool zero_found = true;                      

    do {
        cout << instructions;
        cout << "Enter the first fraction to " << op << ":" << endl;
        cin >> num1 >> solidus1 >> denom1;        
        cout << "Enter the second fraction to " << op << ":" << endl;
        cin >> num2 >> solidus2 >> denom2;
        
        if ((denom1 == 0) || (denom2 == 0)) 
        {
            cout << endl << "Error: denominator can not be zero!" << endl;
            cin.clear();
            cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            zero_found = true;
        }
        else
        {
            zero_found = false; 
            break;    
        }

    } while (zero_found == true);
    
}
IdivideBy0 (27)
Thanks for the input everyone. This is what I came up with while pondering the question and leaving the forum for a bit. It still seems harsh to lose points on a perfectly running program.

Does anyone have advice on when to factor elements of a program into a function? I have a bad feeling I am doomed to repeat the same mistakes over and over (insanity defined!). I hope I'm not out of my league with my desire to become a programmer.

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
 
bool getInputs(int& numerator1, char& solidus1, int& denominator1, int& numerator2, char& solidus2, int& denominator2, char& operation)
{
	
	string strOperation = "";

    switch(operation)
	{
	case '+':
		strOperation = "add :";
		break;
	case '-':
		strOperation = "subtract :";
		break;
	case '*':
		strOperation = "multiply :";
		break;
	case '/':
		strOperation = "divide :";
		break;
	default:
		cout << "Invalid operation selected!";
		return false;
	}
	
	cout << "Data is entered in the following format: \nnumerator/denominator \n" ;
		cout << "Enter first fraction to " << strOperation << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to " << strOperation << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
		
			if((denominator1 == 0) || (denominator2 == 0))  //tests for zero
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				//zero_found = true;
			}

}
Chervil (7320)
That's ok. But I see no reason to have solidus1 and solidus2 as parameters, unless you are going to do something with that value. I assumed it was to be discarded.
IdivideBy0 (27)
I was just looking for a quick and dirty way to answer my own question in Visual Studio 2010, you are correct though, the solidus('s ?) is discarded and therefore not needed in the formal parameter. Also noted the function does NOT return a value as it should.

Now if I can only get my instructor to let me re-submit the assignment.

For those that care?? Here is the complete program. 

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
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
 
#include <iostream>
#include <iomanip>
#include <string>

using namespace std;

void addFractions(int,int,int,int,int&,int&);
void subtractFractions(int,int,int,int,int&,int&);
void multiplyFractions(int,int,int,int,int&,int&);
void divideFractions(int,int,int,int,int&,int&);


void menu();


bool getInputs(int& numerator1, int& denominator1, int& numerator2, int& denominator2, char& operation)
{
	
	string strOperation = "";
	char solidus1, solidus2;

    switch(operation)
	{
	case '+':
		strOperation = "add :";
		break;
	case '-':
		strOperation = "subtract :";
		break;
	case '*':
		strOperation = "multiply :";
		break;
	case '/':
		strOperation = "divide :";
		break;
	default:
		cout << "Invalid operation selected!";
		return false;
	}
	
	cout << "Data is entered in the following format: \nnumerator/denominator \n" ;
		cout << "Enter first fraction to " << strOperation << endl;
		cin >> numerator1 >> solidus1 >> denominator1;		
		cout << "Enter the 2nd fraction to " << strOperation << endl;
		cin >> numerator2 >> solidus2 >> denominator2;
		
			if((denominator1 == 0) || (denominator2 == 0))  //tests for zero
			{
				cout << endl << "Error: denominator can not be zero!" << endl;
				cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				return true;
			}
			else
			{
				return false;
			}

}

int main()
{	
	menu();
	
return 0;
}



void menu()
{
	bool zero_found = false; // Zero based fractions are NOT allowed.
	bool valid = false;	     // Make sure user entered a valid operation.
	char operation = ' ';    // Holds the value of our math operator.
	int numerator1, denominator1, numerator2, denominator2;
	int numAnswer = 0;
	int denAnswer = 0;

	
	
	cout << setw(50) << "Welcome to my fraction calculator program:" << endl << endl;
	cout << "Please select from the following functions:" << endl;
	cout << "'+' To add fractions press + then enter" << endl;
	cout << "'-' To subtract fractions press - then enter" << endl;
	cout << "'*' To multiply fractions press * then enter" << endl;
	cout << "'/' To divide fractions press / then enter" << endl;
	cout << endl << ">>";
	
	do{
	
	cin >> operation;

	if(operation == '+') 
	{
		
		do{
			zero_found = getInputs(numerator1,denominator1,numerator2,denominator2,operation);
			
			if(zero_found != true)
			{
				addFractions(numerator1,denominator1,numerator2,denominator2, numAnswer, denAnswer); // perform calculations
				cout << "The value of these fractions added are: " << numAnswer << "/" << denAnswer << endl;
				valid = true;
			}

		}
		while(zero_found == true);
		
	}
	else if(operation == '-')
	{ 
		do{
			zero_found = getInputs(numerator1,denominator1,numerator2,denominator2,operation);
			
			if(zero_found != true)
			{
				subtractFractions(numerator1,denominator1,numerator2,denominator2, numAnswer, denAnswer); // perform calculations
				cout << "The value of these fractions subtracted are: " << numAnswer << "/" << denAnswer << endl;
				valid = true;
			}

		}
		while(zero_found == true);
		
	}
	else if(operation == '*')
	{
		do{
			zero_found = getInputs(numerator1,denominator1,numerator2,denominator2,operation);
			
			if(zero_found != true)
			{
				multiplyFractions(numerator1,denominator1,numerator2,denominator2, numAnswer, denAnswer); // perform calculations
				cout << "The value of these fractions multiplied are: " << numAnswer << "/" << denAnswer << endl;
				valid = true;
			}

		}
		while(zero_found == true);
	}
	else if(operation == '/')
	{
		do{
			zero_found = getInputs(numerator1,denominator1,numerator2,denominator2,operation);
			
			if(zero_found != true)
			{
				divideFractions(numerator1,denominator1,numerator2,denominator2, numAnswer, denAnswer); // perform calculations
				cout << "The value of these fractions divided are: " << numAnswer << "/" << denAnswer << endl;
				valid = true;
			}

		}
		while(zero_found == true);
	}
	else 
	{
		cout << "Please select a valid operation!" << endl;
	}

	}
	while(!valid);

}

void addFractions(int numerator1, int denominator1, int numerator2, int denominator2, int& numSum, int& denSum)
{
	numSum = numerator1 * denominator2 + numerator2 * denominator1;
	denSum = denominator1 * denominator2;
}

void subtractFractions(int numerator1, int denominator1, int numerator2, int denominator2, int& numSum, int& denSum)
{


	numSum = numerator1 * denominator2 - numerator2 * denominator1;
    denSum = denominator1 * denominator2;


}

void multiplyFractions(int numerator1, int denominator1, int numerator2, int denominator2, int& numSum, int& denSum)
{


	numSum = numerator1 * numerator2;
    denSum = denominator1 * denominator2;

}

void divideFractions(int numerator1, int denominator1, int numerator2, int denominator2, int& numSum, int& denSum)
{


	numSum = numerator1 * denominator2;
    denSum = denominator1 * numerator2;



}
Last edited on
Topic archived. No new replies allowed.