C++

Forum

 
PPP2 Chapter 10 Exercise 3 Help Needed

DragonOsman (715)
I'm having trouble trying to sort a vector of a certain user-defined type that I have (type Reading). I need to sort it to find the median. I tried overloading operators <, >, =, == and !=, but I still get errors. Am I doing it wrong?

Here's the code:
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
 
// temp_stats.cpp : Defines the entry point for the console application.
// Osman Zakir
// 2 / 12 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 10 Exercise 3
// Exercise Specifications:
/**
 * Write a program that reads the data from raw_temps.txt created in exercise 
 * 2 into a vector and then calculates the mean and median temperatures 
 * in your data set. Call this program temp_stats.cpp.
 */

#include "../../std_lib_facilities.h"

struct Reading
{
	int hour;
	double temperature;
	bool operator < (const Reading &reading) const;
	bool operator > (const Reading &reading) const;
	Reading &operator=(const Reading &reading);
	friend bool operator==(const Reading &r1, const Reading &r2);
	friend bool operator!=(const Reading &r1, const Reading &r2);
};

void fill_vector(ifstream &ist, vector<Reading> &temps);
double calc_mean(const vector<Reading> &temps);
double calc_median(const vector<Reading> &temps);
void print_data(const double mean, const double median);

int main()
{
	cout << "Please enter name of input file: ";
	string iname;
	cin >> iname;
	ifstream ist{ iname };
	try
	{
		if (!ist)
		{
			error("can't open input file ", iname);
		}
	}
	catch (const runtime_error &e)
	{
		cerr << "error: " << e.what() << '\n';
	}

	vector<Reading> temps;
	fill_vector(ist, temps);
	double mean = calc_mean(temps);
	double median = calc_median(temps);
	print_data(mean, median);
	keep_window_open();
}

void fill_vector(ifstream &ist, vector<Reading> &temps)
{
	int hour;
	double temperature;
	while (ist >> hour >> temperature)
	{
		try
		{
			if (hour < 0 || hour > 23)
			{
				error("hour out of range");
			}
			temps.push_back(Reading{ hour, temperature });
		}
		catch (const runtime_error &e)
		{
			cerr << "error: " << e.what() << '\n';
		}
	}
}

double calc_mean(const vector<Reading> &temps)
{
	double sum = 0;
	for (const auto &temp : temps)
	{
		sum += temp.temperature;
	}
	double mean = sum / temps.size();
	return mean;
}

double calc_median(const vector<Reading> &temps)
{
	double median = 0;
	sort(temps.begin(), temps.end());
	if (temps.size() % 2 != 0)
	{
		median = temps[temps.size() / 2].temperature;
	}
	else
	{
		long double middle = temps.size() / 2;
		median = double(temps[middle + 1].temperature);
	}
	return median;
}

void print_data(const double mean, const double median)
{
	cout << "The mean is " << mean << " and the median is " << median << '\n';
}

bool Reading::operator < (const Reading &reading) const
{
	return reading.temperature < temperature;
}

bool Reading::operator > (const Reading &reading) const
{
	return reading.temperature > temperature;
}

bool operator==(const Reading &r1, const Reading &r2)
{
	return r1.hour == r2.hour &&
		r1.temperature == r2.temperature;
}

bool operator!=(const Reading &r1, const Reading &r2)
{
	return !(r1 == r2);
}

Reading &Reading::operator=(const Reading &reading)
{
	hour = reading.hour;
	temperature = reading.temperature;

	return *this;
}
Last edited on
Thomas1965 (4571)
On line 92 sort(temps.begin(), temps.end());
You can't sort a const vector since the elements get moved.

On line 99 long double middle = temps.size() / 2;
Why would you want middle as a long double???
It's used as an index and should be a size_t.
DragonOsman (715)
I become an idiot sometimes, I guess. Thanks for pointing those things out.

Edit: I got it to work without any compiler errors, but I still have two "loss of data" warnings on conversion from size_t to unsigned int. I also want to ask if I'm overloading operator< correctly. Here's the code:

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
 
// temp_stats.cpp : Defines the entry point for the console application.
// Osman Zakir
// 2 / 12 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 10 Exercise 3
// Exercise Specifications:
/**
 * Write a program that reads the data from raw_temps.txt created in exercise 
 * 2 into a vector and then calculates the mean and median temperatures 
 * in your data set. Call this program temp_stats.cpp.
 */

#include "../../std_lib_facilities.h"

struct Reading
{
	int hour;
	double temperature;
	Reading &operator=(const Reading &reading);
	bool operator<(const Reading &reading) const;
	friend bool operator==(const Reading &r1, const Reading &r2);
	friend bool operator!=(const Reading &r1, const Reading &r2);
};

void fill_vector(ifstream &ist, vector<Reading> &temps);
double calc_mean(const vector<Reading> &temps);
double calc_median(vector<Reading> &temps);
void print_data(const double mean, const double median);

int main()
{
	cout << "Please enter name of input file: ";
	string iname;
	cin >> iname;
	ifstream ist{ iname };
	try
	{
		if (!ist)
		{
			error("can't open input file ", iname);
		}
	}
	catch (const runtime_error &e)
	{
		cerr << "error: " << e.what() << '\n';
	}

	vector<Reading> temps;
	fill_vector(ist, temps);
	double mean = calc_mean(temps);
	double median = calc_median(temps);
	print_data(mean, median);
	keep_window_open();
}

void fill_vector(ifstream &ist, vector<Reading> &temps)
{
	int hour;
	double temperature;
	while (ist >> hour >> temperature)
	{
		try
		{
			if (hour < 0 || hour > 23)
			{
				error("hour out of range");
			}
			temps.push_back(Reading{ hour, temperature });
		}
		catch (const runtime_error &e)
		{
			cerr << "error: " << e.what() << '\n';
		}
	}
}

double calc_mean(const vector<Reading> &temps)
{
	double sum = 0;
	for (const auto &temp : temps)
	{
		sum += temp.temperature;
	}
	double mean = sum / temps.size();
	return mean;
}

double calc_median(vector<Reading> &temps)
{
	double median = 0;
	sort(temps.begin(), temps.end());
	if (temps.size() % 2 != 0)
	{
		median = temps[temps.size() / 2].temperature;
	}
	else
	{
		size_t middle = temps.size() / 2;
		median = double(temps[middle + 1].temperature);
	}
	return median;
}

void print_data(const double mean, const double median)
{
	cout << "The mean is " << mean << " and the median is " << median << '\n';
}

bool operator==(const Reading &r1, const Reading &r2)
{
	return r1.hour == r2.hour &&
		r1.temperature == r2.temperature;
}

bool operator!=(const Reading &r1, const Reading &r2)
{
	return !(r1 == r2);
}

bool Reading::operator<(const Reading &reading) const
{
	return temperature < reading.temperature;
}

Reading &Reading::operator=(const Reading &reading)
{
	hour = reading.hour;
	temperature = reading.temperature;

	return *this;
}


Thanks in advance.
Last edited on
Thomas1965 (4571)
It's normal to make mistakes. It also took me a while to figure it out. The error messages were not helpful.

BTW. It seems you are making good progress, apart from the mistake the code looks good.
DragonOsman (715)
How do I fix those warnings, then? I don't want warnings, especially not the ones about a possible data loss (and I don't want to just turn the warnings off because that'd be cheating).

Do I have the operator< defined correctly, by the way?
jonnin (11331)
possible data loss is generally seen when you dump a double into an integer, losing the fractional part, or put a large integer into a smaller one (say, char = int). Fixed by changing the types of the variables if needed.
Thomas1965 (4571)
1
2
 
size_t middle = temps.size() / 2;
median = temps[middle + 1].temperature;

Be careful, for values < 3 middle+1 is out of bounds.
DragonOsman (715)
@jonnin: I'm apparently trying to convert a variable of size_t to one of unsigned int, but I don't get how to fix that.

@Thomas: I'm not getting a range-error exception, though. Is that just a coincidence?

What would be a better way to find the median of an even number of items?

Edit: Would this be good enough?
1
2
 
size_t middle = temps.size() / 2;
median = 0.5 * (temps[middle].temperature + temps[middle - 1].temperature);


Last edited on
jlb (4973)
I'm apparently trying to convert a variable of size_t to one of unsigned int, but I don't get how to fix that.

A size_t is an implementation defined unsigned type. Which means that it can be any unsigned type, but with today's compilers it is usually an unsigned int or an unsigned long. It appears that your system is using an unsigned long so when you try to implicitly "cast" the size_t to an unsigned int you get the warning. To fix the problem use the correct variable type, in this case use a size_t instead of the unsigned int.

It would be extremely helpful if in future you would post the complete error messages, exactly as they appear in your development environment. These messages have important information embedded within them to help locate and fix the problems.

Now let's look at this snippet:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
double calc_median(vector<Reading> &temps)
{
	double median = 0;
	sort(temps.begin(), temps.end());
	if (temps.size() % 2 != 0)
	{
		median = temps[temps.size() / 2].temperature;
	}
	else
	{
		size_t middle = temps.size() / 2;
		median = double(temps[middle + 1].temperature);
	}
	return median;
}


First there is no reason for the horrible C cast median = double(temps[middle + 1].temperature); since temperature is already a double. Also if you do need to cast you really should be using C++ style casts because casts, especially C style casts, are extremely dangerous when improperly used and C++ style casts are much easier to spot.

As already stated the code in your else statement can try to access your vector out of bounds, but only if you only have 1 element in your vector.

What would be a better way to find the median of an even number of items?

Edit: Would this be good enough?

Probably, yes.

Last edited on
DragonOsman (715)
These are the warning messages (I'm using Visual Studio 2015, by the way):

1>c:\users\osman\programming\visual studio 2015\projects\programming_principles_practice_using_c++\temp_stats\temp_stats\temp_stats.cpp(94): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data
1>c:\users\osman\programming\visual studio 2015\projects\programming_principles_practice_using_c++\temp_stats\temp_stats\temp_stats.cpp(99): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data


And the lines of code I'm getting them on are:
median = temps[temps.size() / 2].temperature;
and
median = 0.5 * (temps[middle].temperature + temps[middle - 1].temperature);

And I've initialized middle like this: size_t middle = temps.size() / 2;

So I'm pretty sure that the problem isn't there. Which is why I'm confused.
Last edited on
jlb (4973)
Okay to help narrow down the problem I suggest you change the code to:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
double calc_median(vector<Reading> &temps)
{
	double median = 0;
	sort(temps.begin(), temps.end());
        size_t temp_size = temps.size();
        size_t middle = temp_size / 2;
        
        if (temp_size % 2 != 0)
	{
		median = temps[middle].temperature;
	}
	else
	{
		median = (temps[middle + 1].temperature) + temps[middle].temperature)) / 2.0;
	}
	return median;
}


Probably won't help but?

DragonOsman (715)
Yeah, it didn't take away the warnings. I still did it, though. I don't know why I wasn't dividing the second one by 2, though.

Here's what that function looks like now:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
double calc_median(vector<Reading> &temps)
{
	sort(temps.begin(), temps.end());
	size_t middle = temps.size() / 2;
	size_t temps_size = temps.size();
	double median = 0;
	if (temps_size % 2 != 0)
	{
		median = temps[middle].temperature;
	}
	else
	{
		median = 0.5 * (temps[middle].temperature + temps[middle - 1].temperature) / 2;
	}
	return median;
}
jlb (4973)
I don't know why I wasn't dividing the second one by 2, though.

Because you were multiplying by .5 instead of dividing by 2 (and you really should have used 2.0).

Did the warnings change at all? If not then perhaps you're not compiling what you think you are.

DragonOsman (715)
So multiplying by 0.5 would be the same as dividing by 2? I'll divide by 2, then. The result is the same either way, though.

Anyway, I'm still getting the same warnings. This is the full output from the compiler:
1>------ Build started: Project: temp_stats, Configuration: Debug x64 ------
1> temp_stats.cpp
1>c:\users\osman\programming\visual studio 2015\projects\programming_principles_practice_using_c++\temp_stats\temp_stats\temp_stats.cpp(96): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data
1>c:\users\osman\programming\visual studio 2015\projects\programming_principles_practice_using_c++\temp_stats\temp_stats\temp_stats.cpp(100): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data
1> temp_stats.vcxproj -> C:\Users\Osman\programming\visual studio 2015\Projects\programming_principles_practice_using_c++\temp_stats\x64\Debug\temp_stats.exe
1> temp_stats.vcxproj -> C:\Users\Osman\programming\visual studio 2015\Projects\programming_principles_practice_using_c++\temp_stats\x64\Debug\temp_stats.pdb (Full PDB)
========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========


And here's my code:
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
 
// temp_stats.cpp : Defines the entry point for the console application.
// Osman Zakir
// 2 / 12 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 10 Exercise 3
// Exercise Specifications:
/**
 * Write a program that reads the data from raw_temps.txt created in exercise 
 * 2 into a vector and then calculates the mean and median temperatures 
 * in your data set. Call this program temp_stats.cpp.
 */

#include "../../std_lib_facilities.h"

struct Reading
{
	int hour;
	double temperature;
	Reading &operator=(const Reading &reading);
	bool operator<(const Reading &reading) const;
	friend bool operator==(const Reading &r1, const Reading &r2);
	friend bool operator!=(const Reading &r1, const Reading &r2);
};

void fill_vector(ifstream &ist, vector<Reading> &temps);
double calc_mean(const vector<Reading> &temps);
double calc_median(vector<Reading> &temps);
void print_data(const double mean, const double median);

int main()
{
	cout << "Please enter name of input file: ";
	string iname;
	cin >> iname;
	ifstream ist{ iname };
	try
	{
		if (!ist)
		{
			error("can't open input file ", iname);
		}
	}
	catch (const runtime_error &e)
	{
		cerr << "error: " << e.what() << '\n';
	}

	vector<Reading> temps;
	fill_vector(ist, temps);
	double mean = calc_mean(temps);
	double median = calc_median(temps);
	print_data(mean, median);
	keep_window_open();
}

void fill_vector(ifstream &ist, vector<Reading> &temps)
{
	int hour;
	double temperature;
	while (ist >> hour >> temperature)
	{
		try
		{
			if (hour < 0 || hour > 23)
			{
				error("hour out of range");
			}
			temps.push_back(Reading{ hour, temperature });
		}
		catch (const runtime_error &e)
		{
			cerr << "error: " << e.what() << '\n';
		}
	}
}

double calc_mean(const vector<Reading> &temps)
{
	double sum = 0;
	for (const auto &temp : temps)
	{
		sum += temp.temperature;
	}
	double mean = sum / temps.size();
	return mean;
}

double calc_median(vector<Reading> &temps)
{
	sort(temps.begin(), temps.end());
	size_t middle = temps.size() / 2;
	size_t temps_size = temps.size();
	double median = 0;
	if (temps_size % 2 != 0)
	{
		median = temps[middle].temperature;
	}
	else
	{
		median = (temps[middle].temperature + temps[middle - 1].temperature) / 2;
	}
	return median;
}

void print_data(const double mean, const double median)
{
	cout << "The mean is " << mean << " and the median is " << median << '\n';
}

bool operator==(const Reading &r1, const Reading &r2)
{
	return r1.hour == r2.hour &&
		r1.temperature == r2.temperature;
}

bool operator!=(const Reading &r1, const Reading &r2)
{
	return !(r1 == r2);
}

bool Reading::operator<(const Reading &reading) const
{
	return temperature < reading.temperature;
}

Reading &Reading::operator=(const Reading &reading)
{
	hour = reading.hour;
	temperature = reading.temperature;

	return *this;
}


Hope you can help me from here.
Last edited on
jlb (4973)
Since there are no conversions on either of those lines then the problem is somewhere else (perhaps in that include file) or you're not actually compiling this file.

I believe that that header file modifies the std::vector::operator[] to add primitive range validation and maybe somewhere in that modification there is a conversion.


What version of that header file are you #including? Do you have a link to where you downloaded that file?

DragonOsman (715)
I just copied it from Stroustrup's website. This is what the preprocessor lines up top say:
1
2
 
#ifndef H112
#define H112 251113L


How could I not be compiling this file? What do you mean?

By the way, the file says in the comments that we won't need it by Chapter 10. Does this mean I'm allowed to stop using it and move to iostream and etc. now if I want?
Last edited on
jlb (4973)
It appears to me that the following code is the potential problem source.

1
2
3
4
5
6
7
8
9
10
 
	T& operator[](unsigned int i) // rather than return at(i);
	{
		if (i<0||this->size()<=i) throw Range_error(i);
		return std::vector<T>::operator[](i);
	}
	const T& operator[](unsigned int i) const
	{
		if (i<0||this->size()<=i) throw Range_error(i);
		return std::vector<T>::operator[](i);
	}


Notice how he is using an unsigned int in these operator[] functions. If your size_t happens to be an unsigned long you could be unknowingly implicitly casting the unsigned long to an unsigned int.
By the way, the file says in the comments that we won't need it by Chapter 10. Does this mean I'm allowed to stop using it and move to iostream and etc. now if I want?

Yes, I would recommend you stop using this header as soon as possible. You will need to then properly #include all the required header files and properly scope the std namespace. You also won't have the crutch of keep_window_open() you'll need to handle this yourself (or copy the function to your own source file).



DragonOsman (715)
I also like the error() function overloads, so I'll take those as well. I'll make my own header file with these functions. I'm not sure when to #include iomanip, though, so that could be a problem.

Anyway, yeah, thanks for letting me see the problem.
jlb (4973)
I'm not sure when to #include iomanip, though, so that could be a problem.

When you need to use the io manipulators.

Anyway, yeah, thanks for letting me see the problem.

By the way when you have a problem like this it is easier to see the problem when you simplify the "calculation" as much as possible. That is the reason I had you separate all individual parts of the "calculation" so I could better see where the problem occurred. Once I determined what part of your code was "responsible" for the error messages and that the messages didn't match that code I knew to look elsewhere for the problem. Since I knew that the supplied header "modified" the std::vector that was the next place to look. This problem is a good example of why the author called his changes a "disgusting macro hack to get a range checked vector" and a reason to avoid modifying standard classes in the future. It is also a very good reminder of why using the correct type of variable is important, especially when it come to the use of implementation defined types like size_t. Now you need to remember that you don't have the safety net of the range checking when using the operator[] on both the std::vector and std::string so you will need to make sure you either use the .at() member function when you need the range checking or do manual range checking when appropriate.



Topic archived. No new replies allowed.