Sign in to follow this  
Sean_Seanston

Copy Constructor with Container of Pointers?

Recommended Posts

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?

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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!

Share this post


Link to post
Share on other sites
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...

Share this post


Link to post
Share on other sites
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[i];
Component *other = c->clone();
std::cout << c << ' ' << typeid(*c).name() << " -> ";
std::cout << other << ' ' << typeid(*other).name() << '\n';
delete other;
}
}

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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]

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
Of course. The CRTP solution is just automating that.


Nice. It's just with my current situation it'd probably be more of a hassle to add the CRTP in at this point.

Quote:
Original post by rip-off
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.


I'm learning a lot from this project ^_^. It's actually the first time I've bothered making copy constructors...

Quote:
Original post by rip-off
No, sounds reasonable to me.


Alright, I'll do it that way then. Should be no problems from here on.

Thanks all.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
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.

Note that this will change in the next version of the C++ standard, which has quite a few changes for constructors in store. C++0x allows for constructors to call peer constructors, but such calls must be explicit. By default no constructor will call other constructors, you have to tell it to call another constructor in the member initialization list.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this