ansaurus

Question

Assuring proper maintenance of Clone() in C++

Answer 1

+4 A:

Why not just use the copy constructor?

virtual Foo* Clone() const { return new Foo(*this); }

Edit: Oh wait, is Foo the BASE class, rather than the DERIVED class? Same principle.

virtual Foo* Clone() const { return new MyFavouriteDerived(*this); }

Just ensure that all derived classes implement clone in this way, and you won't have a problem. Private copy constructor is no problem, because the method is a member and thus can access it.

DeadMG 2010-07-01 22:32:04

I sometimes use a protected/private copy constructor to do the implementation of clone, but a copy constructor is not polymorphic (the client must know the type of the object to copy) and it just pushes the burden of the one method that must know about (and be maintained simultaneously with) all the data members of the class to the copy constructor method.

David Gladfelter 2010-07-01 22:34:43

@David: No. The copy constructor call is within the virtual method- it goes up the chain, to Foo, and Foo knows it's own type. In addition, if someone maintains a class and doesn't update the copy constructor, it's time for them to be fired or get some more training.

DeadMG 2010-07-01 22:37:49

I'm up-voting this. This method should work. For more information: http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.8 .

Dragontamer5788 2010-07-01 22:47:14

I agree that the copy constructor is more noticable or salient for maintainers' eyeballs. I just wish there were a fool-proof way. This kind of error is easy to commit unless you're looking for it. I know that my own intuition is that adding a new member and new operations based on that member to a class is one of the safest changes you can make to existing code, so anything that goes against that intuition is a likely source of errors in the long run.

David Gladfelter 2010-07-01 22:49:59

@dragon - does anyone actually do it any other way? BTW, beware of covariant return types and multiple inheritance. At least one common compiler goes bat-guano insane.

Noah Roberts 2010-07-01 22:58:53

@David. You can potentially rely on the default copy constructor, which calls the copy constructor of each of the members. If you code in such a way that the default copy constructor is what you need, then you won't have to maintain a manual copy constructor.@Noah, no, I can't think of any other way really.

Dragontamer5788 2010-07-02 04:17:24

Answer 2

+1 A:

Ok, this question has me confused. Initially I thought you wanted to automatically create clone() and I was going to direct you to a discussion I was in a couple years back.

However...that doesn't seem like what you want. So I'll just address your assumptions:

Whenever anyone adds a data member to this class, they've got to remember to update the Clone() method.

Why?? Surely you've implemented clone() in terms of the copy constructor.

struct X { X* clone() const { return new X(*this); } };

Unit tests won't find the error unless they are updated or the class has an equality comparison method and that is both unit tested and was updated to compare the new member.

This shouldn't be a problem since you've surely updated the unit test BEFORE adding the member...

Does anyone else have a problem with Clone() for this reason?

No, I can say with certainty that I've never had your particular problems with clone().

Noah Roberts 2010-07-01 22:37:49

Someone has to know about the data members when copying. If you rely on the compiler-generated copy constructor, raw pointers will be shallow-copied, resulting in errors if they are deleted in the destructor of the class. Are you saying the real solution is to make sure all members have value semantics (e.g. wrap raw pointers to polymorphic objects in smart pointers that use a clone method on the wrapped pointer when their copy constructors are executed) such that the compiler-generated copy constructor will do the right thing?

David Gladfelter 2010-07-01 22:45:31

Who said anything about relying on the compiler-generated copy constructor??? That's a completely independent issue.

Noah Roberts 2010-07-01 22:55:34

Not a problem (yet), just a concern. I'm maintaining someone else's class that has like 20 data members. I am the one implementing Clone(). Some of the classes in this project could really use a refactoring (a lot of dumb value-holder classes), but in the short run my goal is to not make things worse. I agree that an explicit copy constructor is more likely to be properly maintained than a Clone() method. I was just hoping there was a more fool-proof way. The smart pointer approach I thought of in response to your comments might be the way the appeals most to me.

David Gladfelter 2010-07-01 23:09:09

Let me get this straight...you're maintaining a 20 element class that requires a non-trivial copy-constructor but it doesn't have one...and you're implementing clone() for this class. That about right? My advice: it's time for manslaughter.

Noah Roberts 2010-07-01 23:13:49

LOL, @Noah, missed my opportunity on that one since we work for different companies now. In the defense of the code, there was no need for copying before new requirements came in, everything used to be constructed from de-serializing an XML file. There's definitely an "opportunity" for a good deal of time spent refactoring.

David Gladfelter 2010-07-01 23:20:43

@David: Yes. Yes, dear God, don't EVER use raw pointers. You're begging for problems. If your class doesn't have the explicit purpose of dealing with X resource, then you should never have a destructor.

DeadMG 2010-07-02 05:42:04

There are two very reasonable reasons to have a destructor. 1) you're making a new base class. 2) You have done the right thing and only forward declared anything you hold by pointer or reference. Even if you're using smart pointers you'll need to declare a destructor and implement it where these types are fully defined; only the shared pointer can do otherwise and it's not the right one for the purpose of holding a local pointer.

Noah Roberts 2010-07-02 16:09:12

ansaurus

tags:

views:

answers:

Assuring proper maintenance of Clone() in C++

related questions