ICloneable, a perspective on the current guidelines

|

To be talking about an interface this old might seem strange, but I’m primarily working with legacy code (code without tests) at the moment, and I’ve seen some uses of this interface that took me aback, as for me, the thinking shown below has been with me so long it’s almost a subconscious thought; but I felt the need to say it out loud as it might help someone.

First off, ICloneable is considered a Bad ThingTM; the reason for this is poor naming in relation to whether the clone is a deep or shallow copy, see here for more information about this, but here’s a snippet from BradA’s blog about the use of ICloneable:

Because the interface contract does not specify the type of clone performed, different classes have different implementations. A consumer cannot rely on ICloneable to let them know whether an object is deep-cloned or not.

Having said that, cloning is useful. Given that we’re not going to be able to change the name of the interface method inside the framework, here are my guidelines for working with cloning and the ICloneable interface:

  1. Do use the ICloneable interface .. but decide upfront in your project whether Clone means a deep or shallow copy .. to be perfectly honest a shallow copy is rarely useful, therefore I nearly always stipulate that Clone means deep; and then, be consistent throughout the code-base.
  2. If Clone means shallow in your project, then use the Brad’s guideline and create a new interface, say ICloneable<T>, with two methods: Clone and DeepClone.
  3. If following step 1, implement the Clone method to be type safe and then also provide a public typed version of the call; for example:

public class EffectInfo : ICloneable

    #region ICloneable Members

    // Typed version of the call
   
public EffectInfo Clone() 
    {
        return (EffectInfo)this.MemberwiseClone(); 
    }

    // Interface implementation
   
object ICloneable.Clone()
    {
        return this.Clone();
    }

    #endregion
}

<digression> The ICloneable interface shipped with v.1.0 of the Framework, generics did not yet exist; therefore to implement a type safe version of an interface you have to do interface implementation and then provide public typed versions of the members, as shown above. </digression>

Obviously, if you have the concept of ICloneable<T> in your project, with deep and shallow copies, you do not need to take this approach to get type safe versions of the call.

As usual if you have any comments, questions, flames, enhancements I would love to hear from you. In the meantime think deeply and enjoy.

2 comments:

alexey-rom said...

Instead of writing shallow cloning code (as in your EffectInfo example) for all types, why not write it once:

internal T ShallowClone<T>(this T obj) where T : class {
return (T) obj.MemberwiseClone();
}

Paul said...

Hey alexy-rom, thanks for you comment. You're absolutely right you suggest a perfectly vaild approach.

However, to answer your question of why I would not write that code in this scenario (a brown field app) has a number of answers, but simply: 1) assumes a base class to "write it once", which might not be possible or appropriate, 2) convention of ICloneable, if code is already in place with a project .. for a green field app I would take a different approach as suggested in the post with ICloneable<T>.

For places where I would use the approach you've suggested that would have to be a green field app, where a common base class for clonable types was required.

I hope that makes sense, and thanks for your thoughts and for reading.

Kind regards -PJ