C++

Forum

 
 
optimize working code

gongong (80)
hello! my code is working. I just need help optimizing it, e.g. make things shorter, more readable, etc.
The main concern is if there's a way to optimize the function printMonth?

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
 
#include <iostream>
#include <string>
#include <iomanip>
using namespace std;
void printMonth(int month);
void table(double currRainfall[], double prevRainfall[]);
int main()
{
	double rain[12];
	double ave[12];
	int count = 0;

	cout << "\nAverage monthly rainfall" << endl; //ask user to enter monthly rainfall
	for (int i = 0; i < 12; i++)
	{
		printMonth(i);
		cout << ": ";
		cin >> ave[i];
	}

	int currMonth = inputInteger("\nEnter current month: ", 1, 12);

	cout << "\nEnter monthly rainfall from last year" << endl;
	for (int i= currMonth - 1; count < 12; i= (i+ 1) % 12, count++)
	{
		printMonth(i);
		cout << ": ";
		cin >> rain[month];
	}
	cout << endl;

	table(rain, ave);
}
void printMonth(int month)
{
	switch (month)
	{
	case 0:
		cout << "Jan";
		break;
	case 1:
		cout << "Feb";
		break;
	case 2:
		cout << "Mar";
		break;
	case 3:
		cout << "Apr";
		break;
	case 4:
		cout << "May";
		break;
	case 5:
		cout << "Jun";
		break;
	case 6:
		cout << "Jul";
		break;
	case 7:
		cout << "Aug";
		break;
	case 8:
		cout << "Sept";
		break;
	case 9:
		cout << "Oct";
		break;
	case 10:
		cout << "Nov";
		break;
	case 11:
		cout << "Dec";
		break;
        default: break;
	}
}
void table(double currRainfall[], double prevRainfall[])
{
	const string month[12] = { "January", "February", "March",
						 "April", "May", "June",
						"July", "August", "September",
						"October", "November", "December" };

	cout << setw(5) << "Month"
		<< setw(20) << "Current RF"
		<< setw(15) << "Previous RF"
		<< setw(15) << "Average RF"
		<< setw(15) << "Change RF"
		<< endl << left;

	for (int i = 0; i < 70; i++)
	{
		cout << "=";
	}
	cout << endl;

	for (int i = 0; i < 12; i++)
	{
		cout << setw(20) << month[i]
			<< setw(15) << currRainfall[i]
			<< setw(15) << prevRainfall[i]
			<< setw(15) << (currRainfall[i] + prevRainfall[i]) / 2
			<< setw(15) << currRainfall[i] - prevRainfall[1] << endl;
	}
	cout << endl;
}
Last edited on
lastchance (6980)
Use an array. Like the one you use in void table().
Last edited on
gongong (80)
I tried doing that, but the months wouldn't print out.
jonnin (11410)
al that switch etc just becomes
string months[] = {"Jan", "Feb", "Mar", … };
cout << months[1]; //ideally enum match the string?

switches should always have a default.

most of the other things you should do … you do not know enough to do yet. Such as making your own type to hold a month and rain amount, keeping them together.
Last edited on
lastchance (6980)
gongong wrote:
hello! my code is working.

You are joking!
gongong (80)
@jonnin I’ll try that in a bit. Thanks!!

@lastchance it may not work when you run it as I have a header file to validate numeric values.
gongong (80)
@lastchance i just updated it — still didn’t put the header file. no need to be so rude about it
lastchance (6980)
@gongong,
Please upload the code that compiles and does what you says it does, so that people can try it in the on-line compiler.

At the point of my writing this post - and before you update it again - then the online compiler gives the following errors. You can try it yourself by pressing the little gearwheel icon at the top right of your code sample.

In function 'int main()':
21:63: error: 'inputInteger' was not declared in this scope
28:15: error: 'month' was not declared in this scope


Yes, I am well aware how to correct those, but I think it better if you do so first.
Last edited on
zapshe (1870)
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
 
void printMonth(int month)
{
	switch (month)
	{
	case 0:
		cout << "Jan";
		break;
	case 1:
		cout << "Feb";
		break;
	case 2:
		cout << "Mar";
		break;
	case 3:
		cout << "Apr";
		break;
	case 4:
		cout << "May";
		break;
	case 5:
		cout << "Jun";
		break;
	case 6:
		cout << "Jul";
		break;
	case 7:
		cout << "Aug";
		break;
	case 8:
		cout << "Sept";
		break;
	case 9:
		cout << "Oct";
		break;
	case 10:
		cout << "Nov";
		break;
	case 11:
		cout << "Dec";
		break;
        default: break;
	}
}


Can Be Replaced With:

1
2
3
4
5
6
7
8
9
 
void printMonth(int num)
{
     const string month[12] = { "January", "February", "March",
						 "April", "May", "June",
						"July", "August", "September",
						"October", "November", "December" };

     std::cout << month[num]; //No need to change "num" since you already expect it to begin at 0
}
gongong (80)
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
 
#include <iostream>
#include <string>
#include <iomanip>
using namespace std;
void printMonth(int month);
void table(double currRainfall[], double prevRainfall[]);

int main()
{
	double rain[12];
	double ave[12];
	int count = 0;

	cout << "\nEnter average monthly rain" << endl;
	for (int i = 0; i < 12; i++)
	{
		printMonth(i);
		cout << ": ";
		cin >> ave[i];
	}

	int currMonth = inputInteger("\nEnter current month: ", 1, 12);

	cout << "\nEnter monthly rain from last year" << endl;
	for (int month = currMonth - 1; count < 12; month = (month + 1) % 12, count++)
	{
		printMonth(month);
		cout << ": ";
		cin >> rain[month];
	}
	cout << endl;

	table(rain, ave);

	return 0;
}

void printMonth(int month)
{
	//switch (month)
	//{
	//case 0:
	//	cout << "Jan";
	//	break;
	//case 1:
	//	cout << "Feb";
	//	break;
	//case 2:
	//	cout << "Mar";
	//	break;
	//case 3:
	//	cout << "Apr";
	//	break;
	//case 4:
	//	cout << "May";
	//	break;
	//case 5:
	//	cout << "Jun";
	//	break;
	//case 6:
	//	cout << "Jul";
	//	break;
	//case 7:
	//	cout << "Aug";
	//	break;
	//case 8:
	//	cout << "Sept";
	//	break;
	//case 9:
	//	cout << "Oct";
	//	break;
	//case 10:
	//	cout << "Nov";
	//	break;
	//case 11:
	//	cout << "Dec";
	//	break;
	//}
}
void table(double currRain[], double prevRain[])
{
	const string monthName[12] = { "January", "February", "March",
						 "April", "May", "June",
						"July", "August", "September",
						"October", "November", "December" };

	cout << setw(5) << "Month"
		<< setw(20) << "Current RF"
		<< setw(15) << "Previous RF"
		<< setw(15) << "Average RF"
		<< setw(15) << "Change RF"
		<< endl << left;

	for (int i = 0; i < 70; i++)
	{
		cout << "=";
	}
	cout << endl;

	for (int i = 0; i < 12; i++)
	{
		cout << setw(20) << monthName[i]
			<< setw(15) << currRain[i]
			<< setw(15) << prevRain[i]
			<< setw(15) << (currRain[i] + prevRain[i]) / 2
			<< setw(15) << currRain[i] - prevRain[1] << endl;
	}
	cout << endl;
}


here's the header file

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
int inputInteger(string prompt, int startRange, int endRange)
{

	int input;
	do
	{
		cout << prompt;
		if (!(cin >> input))
		{
			cout << "ERROR-3A: Invalid input. Must be an integer type.\n";
			cin.clear();
			cin.ignore(999, '\n');
		}
		else if (!(input >= min(startRange, endRange) && input <= max(startRange, endRange)))
			cout << "ERROR-3A: Invalid input. Must be from " << startRange << "..." << endRange << ".\n";
		else
			break;
	} while (true);
	return input;
}


@lastchange would you like to see the whole project solution?
Last edited on
gongong (80)
@jonnin and @zaphse enum worked perfectly! I'm curious though if there's a way that we can just use one string for the months?
zapshe (1870)
I'm curious though if there's a way that we can just use one string for the months?

Of course, just make the string in the global space so it can be accessed by all functions. Or you do it in int main and provide it as an argument for every function that needs it.
gongong (80)
Thanks, people!
jonnin (11410)
if all the strings have 3 letters, you could shove it into one string and pull what you want back out. This is one of the handful of places a c-string can be a bonus over c++ string -- multiple end of string markers and a fixed (can be calculated) offset... but that is not worth doing without a VERY good justification. The array of strings is correct, one string is not a robust, good design.

if you mean one array of strings for the whole program, you could, and should do that as said by Zapshe.


Topic archived. No new replies allowed.