Proper way to implement ICloneable

What is the proper way of implementing ICloneable in a class hierarchy? Say I have an abstract class DrawingObject. Another abstract class RectangularObject inherits from DrawingObject. Then there are multiple concrete classes like Shape, Text, Circle etc. that all inherit from RectangularObject. I want to implement ICloneable on DrawingObject and then carry it down the hierarchy, copying available properties at each level and calling parent's Clone at the next level.

The problem however is that since the first two classes are abstract, I cannot create their objects in the Clone() method. Thus I must duplicate the property-copying procedure in each concrete class. Or is there a better way?

Solution 1:

You can easily create a superficial clone with object's protected method MemberwiseClone.

Example:

   public abstract class AbstractCloneable : ICloneable
   {
      public object Clone()
      {
         return this.MemberwiseClone();
      }
   }

If you don't need anything like a deep copy, you will not have to do anything in the child classes.

The MemberwiseClone method creates a shallow copy by creating a new object, and then copying the nonstatic fields of the current object to the new object. If a field is a value type, a bit-by-bit copy of the field is performed. If a field is a reference type, the reference is copied but the referred object is not; therefore, the original object and its clone refer to the same object.

If you need more intelligence in the cloning logic, you can add a virtual method to handle references :

   public abstract class AbstractCloneable : ICloneable
   {
      public object Clone()
      {
         var clone = (AbstractCloneable) this.MemberwiseClone();
         HandleCloned(clone);
         return clone;
      }

      protected virtual void HandleCloned(AbstractCloneable clone)
      {
         //Nothing particular in the base class, but maybe useful for children.
         //Not abstract so children may not implement this if they don't need to.
      }
   }


   public class ConcreteCloneable : AbstractCloneable
   {
       protected override void HandleCloned(AbstractCloneable clone)
       {
           //Get whathever magic a base class could have implemented.
           base.HandleCloned(clone);

           //Clone is of the current type.
           ConcreteCloneable obj = (ConcreteCloneable) clone;

           //Here you have a superficial copy of "this". You can do whathever 
           //specific task you need to do.
           //e.g.:
           obj.SomeReferencedProperty = this.SomeReferencedProperty.Clone();
       }
   }

Solution 2:

Give your base class a protected and overridable CreateClone() method that creates a new (empty) instance of the current class. Then have the Clone() method of the base class call that method to polymorphically instantiate a new instance, which the base class can then copy its field values to.

Derived non-abstract classes can override the CreateClone() method to instantiate the appropriate class, and all derived classes that introduce new fields can override Clone() to copy their additional field values into the new instance after invoking the inherited version of Clone().

public CloneableBase : ICloneable
{
    protected abstract CloneableBase CreateClone();

    public virtual object Clone()
    {
        CloneableBase clone = CreateClone();
        clone.MyFirstProperty = this.MyFirstProperty;
        return clone;
    }

    public int MyFirstProperty { get; set; }
}

public class CloneableChild : CloneableBase
{
    protected override CloneableBase CreateClone()
    {
        return new CloneableChild();
    }

    public override object Clone()
    {
        CloneableChild clone = (CloneableChild)base.Clone();
        clone.MySecondProperty = this.MySecondProperty;
        return clone;
    }

    public int MySecondProperty { get; set; }
}

If you want to skip the first overriding step (at least in the default case), you can also suppose a default constructor signature (e.g. parameterless) and try to instantiate a clone instance using that constructor signature with reflection. Like this, only classes whose constructors do not match the default signature will have to override CreateClone().

A very simple version of that default CreateClone() implementation could look like this:

protected virtual CloneableBase CreateClone()
{
    return (CloneableBase)Activator.CreateInstance(GetType());
}

Solution 3:

I believe I have an improvement over @johnny5's excellent answer. Simply define copy constructors in all classes and in the base class use reflection in the Clone method to find the copy constructor and execute it. I think this is slightly cleaner as you don't need a stack of handle clone overrides and you don't need MemberwiseClone() which is simply too blunt an instrument in many situations.

public abstract class AbstractCloneable : ICloneable
    {
        public int BaseValue { get; set; }
        protected AbstractCloneable()
        {
            BaseValue = 1;
        }
        protected AbstractCloneable(AbstractCloneable d)
        {
            BaseValue = d.BaseValue;
        }

        public object Clone()
        {
            var clone = ObjectSupport.CloneFromCopyConstructor(this);
            if(clone == null)throw new ApplicationException("Hey Dude, you didn't define a copy constructor");
            return clone;
        }

    }


    public class ConcreteCloneable : AbstractCloneable
    {
        public int DerivedValue { get; set; }
        public ConcreteCloneable()
        {
            DerivedValue = 2;
        }

        public ConcreteCloneable(ConcreteCloneable d)
            : base(d)
        {
            DerivedValue = d.DerivedValue;
        }
    }

    public class ObjectSupport
    {
        public static object CloneFromCopyConstructor(System.Object d)
        {
            if (d != null)
            {
                Type t = d.GetType();
                foreach (ConstructorInfo ci in t.GetConstructors())
                {
                    ParameterInfo[] pi = ci.GetParameters();
                    if (pi.Length == 1 && pi[0].ParameterType == t)
                    {
                        return ci.Invoke(new object[] { d });
                    }
                }
            }

            return null;
        }
    }

Finally let me speak out in favor of ICloneable. If you use this interface you will get beat up by the style police because the .NET Framework Design Guidelines say not to implement it because, to quote the guidelines, "when using an object that implements a type with ICloneable, you never know what you are going to get. This makes the interface useless." The implication is that you don't know whether you are getting a deep or shallow copy. Well this is simply sophistry. Does this imply that copy constructors should never be used because "you never know what you are going to get?" Of course not. If you don't know what you are going to get, this is a simply a problem with the design of the class, not the interface.

Solution 4:

In order to create deep clone object with new references and avoid mutations on your objects in the most unexpected places, use Serialize/Deserialize.

It will allow full control of what can be cloned (using ignore attribute). Here some example with both System.Text.Json and Newtonsoft.

// System.Text.Json
public object Clone()
{
    // setup
    var json = JsonSerializer.Serialize(this);

    // get
    return JsonSerializer.Deserialize<MyType>(json);
}

// Newtonsoft
public object Clone()
{
    // setup
    var json = JsonConvert.SerializeObject(this);

    // get
    return JsonConvert.DeserializeObject<MyType>(json);
}

// Usage
MyType clonedMyType = myType.Clone();