My move contructor correct?

I could only find simple examples to check against. So I need someone to tell me if I did it right in my case. It compiles, but currently not testable. Thanks.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
class Location {
protected:
	int dimension_x, dimension_y, dimension_z;
	std::vector<std::vector<std::vector<Person*>>> personLocatedAt;
	NamesBinaryTree allPeoplePresentTree;
	std::list<Person*> allPeoplePresent;
	std::list<std::shared_ptr<Crowd>> crowds;
public:
	Location () = default;
	Location (const Location&) = default;
	Location (Location&& loc) noexcept: dimension_x (loc.dimension_x), dimension_y (loc.dimension_y), dimension_z (loc.dimension_z), personLocatedAt (std::move (loc.personLocatedAt)),
		allPeoplePresentTree (std::move (loc.allPeoplePresentTree)), allPeoplePresent (std::move (loc.allPeoplePresent)), crowds (std::move (loc.crowds)) {
			for (auto& x: loc.personLocatedAt)
				for (auto& y: x)
				 	for (auto& z: y)
						z = nullptr;
			for (std::list<Person*>::iterator it = loc.allPeoplePresent.begin(); it != loc.allPeoplePresent.end(); ++it)
				*it = nullptr;
			for (std::list<std::shared_ptr<Crowd>>::iterator it = loc.crowds.begin(); it != loc.crowds.end(); ++it)
				*it = nullptr;
			// loc.allPeoplePresentTree contains no pointers to set to nullptr
	}
};
Last edited on
> but currently not testable
so make a test

¿couldn't you simply `clear()' the lists ? that way it would be O(1) instead of O(n)
Why are you overwriting all of the data with nullptr?
-> Why are you overwriting all of the data with nullptr?

Ah! That was what I was suspicious about. The simple examples I could find were setting the pointers to nullptr, so I was following the idea. But since the only goal is to avoid the crash when the other Location object is destroyed, then clearing the containers should be good enough. Didn't think of that.

-> couldn't you simply `clear()' the lists ? that way it would be O(1) instead of O(n)
I'm glad I didn't write the code to test it. Because I probably woudn't have noticed that it was O(n) instead of O(1), and thus I would have falsely concluded that it is doing the job right.
Last edited on
There is no reason to clear() the lists or overwrite the data. std::vector and std::list do the correct thing when moved. Whether NamesBinaryTree supports move semantics correctly is the only thing I would be worried about here.
Last edited on
Why write this at all? It's not doing anything differently from the default move constructor (elementwise move, and then some code that does nothing because moved-from containers were just emptied)
Keep it simple, follow the Rule of Zero.
Topic archived. No new replies allowed.