Copy Constructor with Container of Pointers?

Started by
10 comments, last by SiCrane 13 years, 4 months ago
I have to make a Copy Constructor that copies an object with a map containing pointers with std::strings for keys. So... as you can imagine, a simple map1 = map2 isn't what I'm after since that results in both sets of pointers pointing to the same objects.

What I need is to copy the key of each pair of the 2nd map into the 1st along with a pointer to a newly-created copy of the object pointed to by the pointer.

I've trying to come up with something but it's making my head hurt and I'm not sure that I understand the situation well enough. I figure similar problems must've come up before often enough so I'd ask here while I try to figure it out.
Anyone able to suggest a solution?
Advertisement
Sounds like you simply need to, for each key in the old map, invoke the copy constructor of the value for that key, and insert the key and copied value into the new map. Assuming the value type has a copy constructor, of course, and you're storing pointers to them: T* copy = new T(original); should work for creating the copy of the value.

Why are you storing pointers?
Well, that's easy -- it's just what std::transform is for.

#include <string>#include <map>#include <iterator>#include <utility>#include <algorithm>template<typename T> struct DeepCopy {	T* operator()(T const* p) {		return new T(*p); 	}};template<typename InnerTransform> struct ValueTransform{	ValueTransform(InnerTransform it) : it(it) { }	template<typename T1, typename T2>	std::pair<T1, T2> operator()(std::pair<T1, T2> const& x) { 		return std::pair<T1, T2>(x.first, it(x.second));	}	InnerTransform it;};template<typename InnerTransform> ValueTransform<InnerTransform> MakeValueTransform(InnerTransform it) {	return ValueTransform<InnerTransform>(it);}int main(){	std::map<std::string, int*> oldmap, newmap;	oldmap["foo"] = new int(3);	oldmap["bar"] = new int(4);	std::transform(		oldmap.begin(), oldmap.end(), 		std::insert_iterator<std::map<std::string, int*> >(newmap, newmap.end()),		MakeValueTransform(DeepCopy<int>()));}


Whee!
Or use something like mr edd's value_ptr class.
Quote:Original post by jpetrie
Sounds like you simply need to, for each key in the old map, invoke the copy constructor of the value for that key, and insert the key and copied value into the new map. Assuming the value type has a copy constructor, of course, and you're storing pointers to them: T* copy = new T(original); should work for creating the copy of the value.


Aha... makes sense.

Quote:Original post by jpetrieWhy are you storing pointers?


Well, it's a map inside an Entity class for a basic component system. I need pointers for polymorphism for storing Component*.

In fact, the only reason I want a copy constructor is because I'm doing a college assignment that appears need copy constructors for classes.

So... I need to create a new Component* that points to a newly created Component of whatever type the old one was pointing to. Each component type has a string that identifies its type... would I pretty much have to use some kind of switch statement to create the new object?

I really should've mentioned the polymorphism in my original post... the significance slipped my mind.

Sneftel, would that still be good for what I described? I'll have to go research std::transform, I've never used it before...
You could use a virtual clone() method. Each derived class could override it to return a copy of itself.

You could use CRTP to make it easier:
class Component {public:    virtual ~Component() {}    virtual Component *clone() = 0;};template<class T>class ComponentCRTP : public Component {   virtual Component *clone() {        T *self = static_cast<T*>(this);        return new T(*self);   }};class SomeComponent : public ComponentCRTP<SomeComponent> {   // ...};class OtherComponent : public ComponentCRTP<OtherComponent > {   // ...};#include <iostream>#include <vector>int main(){   std::vector<Component *> components;   SomeComponent some;   OtherComponent other;   components.push_back(&some);   components.push_back(&other);   for(unsigned i = 0 ; i < components.size() ; ++i )   {      Component *c = components;      Component *other = c->clone();      std::cout << c << ' ' << typeid(*c).name() << " -> ";      std::cout << other << ' ' << typeid(*other).name() << '\n';      delete other;   }}
Quote:Original post by Sean_Seanston
Sneftel, would that still be good for what I described?
If modified to use rip-off's virtual clone method rather than new T, it would still be applicable. But I was only half-serious -- that forest of templates and SC++L stuff is much more complicated than the mostly* equivalent simple for-loop:

for(map<...>::const_iterator i=oldmap.begin(); i != oldmap.end(); ++i) {  newmap[i->first] = i->second->clone();}


*"mostly" because the wordy approach has somewhat better performance guarantees, but they're unlikely to make a difference, and replicating them in the for-loop makes it only marginally more complex.
Cool, I've a much better idea about it now, thanks.

One question though after I've Google'd this a bit:

Is there a reason you can't just make a function in the base class like:

virtual Component* clone();


Then in a derived class go:

Derived* clone(){return new Derived( *this )}


?

Provided you have a copy constructor set up properly.
I saw that or something like it on another site so I'm wondering if it's possible to dispense with the extra class and level of inheritance.

EDIT: Just tried that and it seems to be working. Still worried there might be something I'm missing though >_>.
EDIT2: Actually there are some problems with it now I see, I need to figure it out though...

EDIT3: Hmm... I think it's working except that I forgot to set the parent variable of the new components to the new Entity so they're pointing to the old one still. Bah. That wrecked my head for ages. At least I think/hope that's it.

EDIT4: Wow, also turned out that an object's constructor isn't called before the copy constructor... I would've thought it would be called too o_o.

[Edited by - Sean_Seanston on December 8, 2010 5:38:43 PM]
Alright, everything's working now except for one thing.

Til now, I made it so when a CollisionComponent was created, it would register its parent Entity with the Collision Manager. That worked fine but now things are different since when a Copy Constructor is called by using clone(), a Component is created that still has the original Entity as its parent. The proper new Entity is only set to be its parent right afterward.

Seemingly, it makes most sense to register with the collision manager inside the copy constructor since that's the counterpart of the original constructor where the same thing happens, but obviously the parent being the old Entity is preventing that.

Do you think it's reasonable that the clone() function should have an Entity* parameter, given that there's really no point in a component existing without a parent Entity, or is that bad ugly practice and might there be a better way?

EDIT: Or of course just overload it and have both ways. Maybe I'll try that.
Quote:
Is there a reason you can't just make a function in the base class like... Then in a derived class go...

Of course. The CRTP solution is just automating that.

Quote:
Wow, also turned out that an object's constructor isn't called before the copy constructor... I would've thought it would be called too o_o.

The copy constructor is a constructor. In C++, constructors don't call each other. At least within a single class they cannot, base class constructors will be invoked implicitly if they are not called via initialiser list.

Quote:
Do you think it's reasonable that the clone() function should have an Entity* parameter, given that there's really no point in a component existing without a parent Entity, or is that bad ugly practice and might there be a better way?

No, sounds reasonable to me.

This topic is closed to new replies.

Advertisement