Copy constructor versus Clone()

In C#, what is the preferred way to add (deep) copy functionality to a class? Should one implement the copy constructor, or rather derive from ICloneable and implement the Clone() method?

Remark: I wrote "deep" within brackets because I thought it was irrelevant. Apparently others disagree, so I asked whether a copy constructor/operator/function needs to make clear which copy variant it implements.

You should not derive from ICloneable.

The reason is that when Microsoft designed the .net framework they never specified whether the Clone() method on ICloneable should be a deep or shallow clone, thus the interface is semantically broken as your callers won't know whether the call will deep or shallow clone the object.

Instead, you should define your own IDeepCloneable (and IShallowCloneable) interfaces with DeepClone() (and ShallowClone()) methods.

You can define two interfaces, one with a generic parameter to support strongly typed cloning and one without to keep the weakly typed cloning ability for when you are working with collections of different types of cloneable objects:

public interface IDeepCloneable
{
    object DeepClone();
}
public interface IDeepCloneable<T> : IDeepCloneable
{
    T DeepClone();
}

Which you would then implement like this:

public class SampleClass : IDeepCloneable<SampleClass>
{
    public SampleClass DeepClone()
    {
        // Deep clone your object
        return ...;
    }
    object IDeepCloneable.DeepClone()   
    {
        return this.DeepClone();
    }
}

Generally I prefer to use the interfaces described as opposed to a copy constructor it keeps the intent very clear. A copy constructor would probably be assumed to be a deep clone, but it's certainly not as much of a clear intent as using an IDeepClonable interface.

This is discussed in the .net Framework Design Guidelines and on Brad Abrams' blog

_{(I suppose if you are writing an application (as opposed to a framework/library) so you can be sure no one outside of your team will be calling your code, it doesn't matter so much and you can assign a semantic meaning of "deepclone" to the .net ICloneable interface, but you should make sure this is well documented and well understood within your team. Personally I'd stick to the framework guidelines.)}

> In C#, what is the preferred way to add (deep) copy functionality to a class? > Should one implement the copy constructor, > or rather derive from ICloneable and implement the Clone() method?

The problem with ICloneable is, as others have mentioned, that it does not specify whether it is a deep or shallow copy, which makes it practically unuseable and, in practice, rarely used. It also returns object, which is a pain, since it requires a lot of casting. (And though you specifically mentioned classes in the question, implementing ICloneable on a struct requires boxing.)

A copy constuctor also suffers from one of the problems with ICloneable. It isn't obvious whether a copy constructor is doing a deep or shallow copy.

Account clonedAccount = new Account(currentAccount); // Deep or shallow?

It would be best to create a DeepClone() method. This way the intent is perfectly clear.

This raises the question of whether it should be a static or instance method.

Account clonedAccount = currentAccount.DeepClone();  // instance method

or

Account clonedAccount = Account.DeepClone(currentAccount); // static method

I slightly prefer the static version sometimes, just because cloning seems like something that is being done to an object rather than something the object is doing. In either case, there are going to be issues to deal with when cloning objects that are part of an inheritence hierarchy, and how those issues are delt with may ultimately drive the design.

class CheckingAccount : Account
{
    CheckAuthorizationScheme checkAuthorizationScheme;

    public override Account DeepClone()
    {
        CheckingAccount clone = new CheckingAccount();
        DeepCloneFields(clone);
        return clone;
    }

    protected override void DeepCloneFields(Account clone)
    {
        base.DeepCloneFields(clone);
        
        ((CheckingAccount)clone).checkAuthorizationScheme = this.checkAuthorizationScheme.DeepClone();
    }
}

I recommend using a copy constructor over a clone method primarily because a clone method will prevent you from making fields readonly that could have been if you had used a constructor instead.

If you require polymorphic cloning, you can then add an abstract or virtual Clone() method to your base class that you implement with a call to the copy constructor.

If you require more than one kind of copy (ex: deep/shallow) you can specify it with a parameter in the copy constructor, although in my experience I find that usually a mixture of deep and shallow copying is what I need.

Ex:

public class BaseType {
   readonly int mBaseField;

   public BaseType(BaseType pSource) =>
      mBaseField = pSource.mBaseField;

   public virtual BaseType Clone() =>
      new BaseType(this);
}

public class SubType : BaseType {
   readonly int mSubField;
   
   public SubType(SubType pSource)
   : base(pSource) =>
      mSubField = pSource.mSubField;

   public override BaseType Clone() =>
      new SubType(this);
}

There is a great argument that you should implement clone() using a protected copy constructor

> It is better to provide a protected (non-public) copy constructor and invoke that from the clone method. This gives us the ability to delegate the task of creating an object to an instance of a class itself, thus providing extensibility and also, safely creating the objects using the protected copy constructor.

So this is not a "versus" question. You may need both copy constructor(s) and a clone interface to do it right.

(Although the recommended public interface is the Clone() interface rather than Constructor-based.)

Don't get caught-up in the explicit deep or shallow argument in the other answers. In the real world it is almost always something in-between - and either way, should not be the caller's concern.

The Clone() contract is simply "won't change when I change the first one". How much of the graph you have to copy, or how you avoid infinite recursion to make that happen shouldn't concern the caller.

Implementing ICloneable's not recommended due to the fact that it's not specified whether it's a deep or shallow copy, so I'd go for the constructor, or just implement something yourself. Maybe call it DeepCopy() to make it really obvious!

You'll run into problems with copy constructors and abstract classes. Imagine you want to do the following:

abstract class A
{
    public A()
    {
    }

    public A(A ToCopy)
    {
        X = ToCopy.X;
    }
    public int X;
}

class B : A
{
    public B()
    {
    }

    public B(B ToCopy) : base(ToCopy)
    {
        Y = ToCopy.Y;
    }
    public int Y;
}

class C : A
{
    public C()
    {
    }

    public C(C ToCopy)
        : base(ToCopy)
    {
        Z = ToCopy.Z;
    }
    public int Z;
}

class Program
{
    static void Main(string[] args)
    {
        List<A> list = new List<A>();

        B b = new B();
        b.X = 1;
        b.Y = 2;
        list.Add(b);

        C c = new C();
        c.X = 3;
        c.Z = 4;
        list.Add(c);

        List<A> cloneList = new List<A>();

        //Won't work
        //foreach (A a in list)
        //    cloneList.Add(new A(a)); //Not this time batman!

        //Works, but is nasty for anything less contrived than this example.
        foreach (A a in list)
        {
            if(a is B)
                cloneList.Add(new B((B)a));
            if (a is C)
                cloneList.Add(new C((C)a));
        }
    }
}

Right after doing the above, you start wishing you'd either used an interface, or settled for a DeepCopy()/ICloneable.Clone() implementation.

The problem with ICloneable is both intent and consistency. It's never clear whether it is a deep or shallow copy. Because of that, it's probably never used in only one manner or another.

I don't find a public copy constructor to be any clearer on that matter.

That said, I would introduce a method system that works for you and relays intent (a'la somewhat self documenting)

If the object you are trying to copy is Serializable you can clone it by serializing it and deserializing it. Then you don't need to write a copy constructor for each class.

I don't have access to the code right now but it is something like this

public object DeepCopy(object source)
{
   // Copy with Binary Serialization if the object supports it
   // If not try copying with XML Serialization
   // If not try copying with Data contract Serailizer, etc
}

It is dependent on copy semantics of the class in question, which you should define yourself as the developer. Chosen method is usually based on intended use cases of the class. Maybe it will make a sense to implement both methods. But both share similar disadvantage - it is not exactly clear which copying method they implement. This should be clearly stated in documentation for your class.

For me having:

// myobj is some transparent proxy object
var state = new ObjectState(myobj.State);

// do something

myobject = GetInstance();
var newState = new ObjectState(myobject.State);

if (!newState.Equals(state))
    throw new Exception();

instead of:

// myobj is some transparent proxy object
var state = myobj.State.Clone();

// do something

myobject = GetInstance();
var newState = myobject.State.Clone();

if (!newState.Equals(state))
    throw new Exception();

looked as clearer statement of intent.

I think there should be a standard pattern for cloneable objects, though I'm not sure what exactly the pattern should be. With regard to cloning, it would seem there are three types of classes:

1. Those that explicitly support for deep cloning
2. Those that where memberwise cloning will work as deep cloning, but which neither have nor need explicit support.
3. Those which cannot be usefully deep cloned, and where memberwise cloning will yield bad results.

So far as I can tell, the only way (at least in .net 2.0) to get a new object of the same class as an existing object is to use MemberwiseClone. A nice pattern would seem to be to have a "new"/"Shadows" function Clone which always returns the present type, whose definition is always to call MemberwiseClone and then call a protected virtual subroutine CleanupClone(originalObject). The CleanupCode routine should call base.Cleanupcode to handle the base type's cloning needs and then add its own cleanup. If the cloning routine has to use the original object, it would have to be typecast, but otherwise the only typecasting would be on the MemberwiseClone call.

Unfortunately, the lowest level of class that was of type (1) above rather than type (2) would have to be coded to assume that its lower types would not need any explicit support for cloning. I don't really see any way around that.

Still, I think having a defined pattern would be better than nothing.

Incidentally, if one knows that one's base type supports iCloneable, but does not know the name of the function it uses, is there any way to reference the iCloneable.Clone function of one's base type?

If you read through all the interesting answers and discussions, you might still ask yourself how exactly you copy the properties - all of them explicitly, or is there a more elegant way to do it? If that is your remaining question, take a look at this (at StackOverflow):

How can I “deeply” clone the properties of 3rd party classes using a generic extension method?

It describes how to implement an extension method CreateCopy() which creates a "deep" copy of the object including all properties (without having to copy property by property manually).