C++

Forum

 
Constructive criticisms on working code

Spiffy P Mcgee (9)
Hello, my program works fine, but I would like to know how I could improve it. Tips on organization would also be welcome. Thanks!
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
 
#include <iostream>
#include <iomanip>
using namespace std;
void menu();
void car(double, double, double, double);
void air(double, double, double, double);
void hel(double, double, double, double);
void hyd(double, double, double, double);
int main()
{
	
	cout << showpoint << fixed << setprecision(1);
	int option;
	double time;
	double distancemeters = 0;
	double distancemiles = 0;
	const double meters_mile = 1609;
	
	cout << "  DISTANCE FROM SOUND IN GASES\n\n";
	
	do
	{
		menu();
		cin >> option;
		cout << endl;
		
	switch (option)
	{
		case 1:
		{	
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		while (time < 0 || time > 30)
		{
			cout << "The number must be between 0 and 30. \n";
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		}

			car(time,  distancemeters, distancemiles, meters_mile);
				break;
		}
		case 2:
		{
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		
		while (time < 0 || time > 30)
		{
			cout << "The number must be between 0 and 30. \n";
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		}
			air(time, distancemeters, distancemiles, meters_mile);
			break;
		}
		case 3:
		{
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		
		while (time < 0 || time > 30)
		{
			cout << "The number must be between 0 and 30. \n";
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
		}
			hel(time, distancemeters, distancemiles, meters_mile);
			break;
		}
		case 4:
		{	
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl;
	
		while (time < 0 || time > 30)
		{
			cout << "The number must be between 0 and 30. \n";
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;
			cout << endl ;
		}

			hyd(time, distancemeters, distancemiles, meters_mile);
			break;
		}
		case 5:
			break;
		default: 	
			cout << "You must select 1, 2, 3, or 4.\n\n";
			break;
		}
	}
	while (option != 5);
		cout << "Programmer: E. R.\n";
	
	return 0;
}
	void menu()
	{
		cout << "1. Distance in carbon dioxide\n";
		cout << "2. Distance in air\n";
		cout << "3. Distance in helium\n";
		cout << "4. Distance in hydrogen\n";
		cout << "5. Quit\n\n";
		cout << "Enter your choice (1-5): ";

	}
		void car(double time, double distancemeters, double distancemiles, double meters_mile)
	{
		const double carbonspeed = 258.0;
		distancemeters = carbonspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in carbon is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}
		
	void air(double time, double distancemeters, double distancemiles, double meters_mile)
	{
		const double airspeed = 331.5;
		distancemeters = airspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in air is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}	
	void hel(double time, double distancemeters, double distancemiles, double meters_mile)
	{
		const double heliumspeed = 972.0;
		distancemeters = heliumspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in helium is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}

			void hyd(double time, double distancemeters, double distancemiles, double meters_mile)
	{
		const double hydrogenspeed = 1270.0;
		distancemeters = hydrogenspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in hydrogen is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}

Philip Lee Bridson (52)
Hi there,

You could try putting this into one or more classes. Although the code works perfectly fine it is not very object-orientated. Try defining two classes - one for the menu handling features and one for the calculations. If you need any help feel free to ask.

Kind Regards,

Phil.
jsmith (5804)
car, air, hel, and hyd differ from one another by exactly one constant. The other 10 lines of the functions are
identical (except for the name of the constant variable and part of the hard-coded string in the cout). Consider
ways to eliminate the redundant code -- it will reduce the length of the program by 25%.
Spiffy P Mcgee (9)
Hey, I'm trying to cut it down. The distancemeters and distancemiles variables are bothering me, I'm trying to find a better way to store that data (maybe reference variables?). I also want to get the "very close, moderately close, far away" bits of the program into another function, so I don't repeat myself. 
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
 
 
#include <iostream>
#include <iomanip>
using namespace std;
void menu();								//menu function
double timefunc();
void car(double, double, double);			//carbon dioxide function
void air(double, double, double);			//air function
void hel(double, double, double);			//helium function
void hyd(double, double, double);			//hydrogen function

const double meters_mile = 1609;			//meters in a mile

int main()
{
	
	cout << showpoint << fixed << setprecision(1);				//1 decimal place shown
	int option;													//menu choice
	double distancemeters = 0;									//distance, in meters
	double distancemiles = 0;									//distance, in miles
	
	cout << "  DISTANCE FROM SOUND IN GASES\n\n";				//Program Purpose
	
	do															//begins loop
	{
		menu();													//menu prompt
		cin >> option;											//user inputs choice
		cout << endl;
		
	switch (option)
	{
		case 1:														//1 computes distance travelled in carbon dioxide
		{	
			double time = timefunc()
			car(time,  distancemeters, distancemiles);				//car function called
				break;
		}
		case 2:														//2 computes distance travelled in air
		{
			double time = timefunc();
			air(time, distancemeters, distancemiles);				//air function called
			break;
		}
		case 3:														//3 computes distance travelled in helium
		{
			double time = timefunc();
			hel(time, distancemeters, distancemiles);				//hel function called
			break;
		}
		case 4:														//4 computes distance travelled in hydrogen
		{	
			double time = timefunc();
			hyd(time, distancemeters, distancemiles);				//hyd function called
			break;
		}
		case 5:														//5 leaves the loop
			break;
		default: 	
			cout << "You must select 1, 2, 3, or 4.\n\n";			//invalid menu option, print error
			break;
		}
	}
	while (option != 5);											//if 5 is chosen, loop ends, else loop begins again
		cout << "Programmer: E.R.\n";
	
	return 0;
}
	
	void menu()														
	{
		cout << "1. Distance in carbon dioxide\n";
		cout << "2. Distance in air\n";
		cout << "3. Distance in helium\n";
		cout << "4. Distance in hydrogen\n";
		cout << "5. Quit\n\n";
		cout << "Enter your choice (1-5): ";

	}

	double timefunc()
	{
			double time;											//sound travel time, in seconds
			cout << "Enter the number of seconds elapsed: ";
			cin >> time;											//input time
			cout << endl;
		
		while (localtime < 0 || localtime > 30)								//test for valid input
		{
			cout << "The number must be between 0 and 30. \n";
			cout << "Enter the number of seconds elapsed: ";
			cin >> localtime;
			cout << endl;
		}
		return time;
		 
	}

		void car(double time, double distancemeters, double distancemiles)
	{																
		const double carbonspeed = 258.0;							//speed of sound in carbon				
		distancemeters = carbonspeed * time;						//computes distance in meters
		distancemiles = distancemeters / meters_mile;				//converts distance in meters to distance in miles
		cout << "The distance travelled in carbon is ";				//prints result
		cout << distancemeters << " meters or " << distancemiles; 
		cout << " miles.\n";
		cout << "The sound source is ";								
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}
		
	void air(double time, double distancemeters, double distancemiles)
	{
		const double airspeed = 331.5;
		distancemeters = airspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in air is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}	
	void hel(double time, double distancemeters, double distancemiles)
	{
		const double heliumspeed = 972.0;
		distancemeters = heliumspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in helium is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}

			void hyd(double time, double distancemeters, double distancemiles)
	{
		const double hydrogenspeed = 1270.0;
		distancemeters = hydrogenspeed * time;
		distancemiles = distancemeters / meters_mile;
		cout << "The distance travelled in hydrogen is " << distancemeters << " meters or " << distancemiles << " miles.\n";
		cout << "The sound source is ";
		if (distancemiles < 1)
			cout << "very close.\n\n";
		else if (distancemiles < 5)
			cout << "moderately close.\n\n";
		else 
			cout << "far away.\n\n";
	}
Topic archived. No new replies allowed.