Should we Obsolete ICloneable (The SLAR on System.ICloneable)


I decided to jump ahead a bit on my series on sharing some of the information in the .NET Framework Standard Library Annotated Reference Vol 1. This week the BCL team spent some time thinking about whether or not we should obsolete the ICloneable and I thought I’d let you folks participate in that debate.


 


 


The Background…


 


From the Design Guidelines:


The ICloneable interface contains a single Clone method, which is used to create a copy of the current object.


public interface ICloneable {


       object Clone();


}


 


Do not implement ICloneable[1].


There are two general ways to implement ICloneable, either as a deep, or non-deep copy. Deep-copy copies the cloned object and all objects referenced by the object, recursively until all objects in the graph are copied. A non-deep copy (referred to as ‘shallow’ if only the top level references are copied) may do none, or part of a deep copy.


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.



Note: If you need a cloning mechanism, define your own Clone, or Copy methodology, and ensure that you document clearly whether it is a deep or shallow copy. An appropriate pattern is:

public <type> Copy();

 


Do not use ICloneable in public APIs 


 


From the .NET Framework Standard Library Annotated Reference Vol 1:


JR The documentation for this interface states that Clone can make either a shallow


or deep copy. This is ridiculous; the standard should have defined Clone as making


a deep copy. Without a guarantee of the type of copy, you must be careful how you


use a cloned object and have intimate knowledge of what Clone does for a type; this


information is usually not documented, leading to problems.


 


KG Echoing Jeffrey’s comment, without a contract for ICloneable, which speci-


fies whether it is deep or shallow, this interface is close to unusable. Defining a deepcopy


version would have definitely been the most useful approach here.


Because of this loose contract, anyone utilizing the ICloneable aspect of a class


really has no idea what he or she can then do with the class. Users can’t assume that


it’s a tearaway version (the result of a deep copy), because if they manipulate the


resulting “Clone” at all, then they may also be affecting the original. This may have


unwanted side effects.


Unfortunately, the disparate implementations of this interface throughout the framework


make it next to impossible to fix at this stage. You can just define a Copy method


on classes in order to support your own deep-cloning; however, I do urge caution


since this can be very difficult to guarantee. As soon as you have a reference to something


which does not itself define a deep-clone mechanism, you cannot successfully


 


 


The Debate


The arguments in favor of obsoleting ICloneable include:


1.       Obsoleting it will help us clean-up our mistake


2.       Obsoleting is much stronger than the design guideline and FxCop rule we have


3.       Obsoleting makes it more clear the developers using the framework what the issues are with this type


 


The argument against obsoleting ICloneable include:


1.       Developers could define a strong contract for ICloneable on their types thereby making this a useful interface.


2.       The Design Guidelines rules and FxCop are good enough form of education on this issue


3.       We should not obsolete this ICloneable until we have a replacement design that will work better.


 


 


As always, I’d love to hear your comments, concerns, thoughts on this issue…




Comments (36)

  1. David M. Kean says:

    How about a IShallowCloneable and a IDeepCloneable with the following methods:

    IShallowCloneable.ShallowClone();

    IDeepCloneable.DeepClone();

    This is how I have implemented my cloning for my libraries.

    I think that ICloneable should not be obsoleted until there is a replacement.

  2. Judah Himango says:

    I’d say obsolete it. The framework is still young, the whole world is not yet using .NET, fix your mistakes while the impact is still relatively small. Obsolete it.

  3. Daniel O'Connell says:

    I agree it needs to be obsoleted, however an alternative needs to be in place. The common choices for non-IClonable cloning are CopyClone methods without a backing interface, custom interfaces, or the most horrid choice: copy constructors. An explicit way to provide a cloning contract is nessecery…more so with the coming of generics. Its unfortunate ICloneable wasn’t right, but there is a need for contractual cloning.

    As for the arguments against obsoleteing,

    1 implies that a custom interface would work just as well. An interface which has a meaning defined at implementatoin and not declaration is loose and will eventually run into issues. If an object is cast to ICloneable, then the rules applied to the type are gone as far as the user goes, he or she may not be able to find out what Clone will do and hence the original problem rears itself here as well. As for 2, this is a situation, IMHO, that there isn’t education. The interface is wrong and a source of bugs and complexity in the framework. It just doesn’t work and there is a price involved.

  4. What about generic cloning semantics? The determination of the best cloning method, deep or shallow, should often be in the class performing the cloning, not in the code calling the clone method. From this perspective, there should only be a single clone method.

    If this method is implemented with shallow cloning, it should behave as if it were a deep clone. That is, owned child objects should be lazily cloned on the fly, as needed, to make the whole hierarchy unique.

    Simply slapping a "deep" or "shallow" label on the method itself is not so useful, except for simple, non-owning collections.

  5. Steve Wart says:

    Yes please, get rid of this and replace it with something simple and usable:

    Make Object.Copy() part of MSCorLib.dll to do a member-wise copy, and create a PostCopyEvent for any "deep" cloning behavior.

    BTW, I like "Copy" better than "Clone", but maybe I’m trying to simplify things too much :-)

  6. Daniel O'Connell says:

    Frank: The problem isn’t so much in the concept as it is in the interface. An interface shouldn’t be so loose that you don’t know what it does in any given case. An implementation of Clone is free, both semantically and contractually, to do a deep clone, a shallow clone, or something in between(deep clone part of the graph and shallow clone the other half). The problem with this is it can lead to ugly patterns where its very difficult to know which portions of the object are safe to modify.

    Another issue you have is, for libraries, its very difficult to determine beforehand how Clone should behave. There will be many situations where both deep and shallow cloning should be exposed, depending entirely on the class usage. Its a mistake to assume a class can always understand the usage scenarios it will participate in to the granularity of cloning. It works ok for some things, but it certainly doesn’t for everything.

    You do have a point, however. There will be cases when the user code simply doesn’t care about the type of cloning and will rely on the class authors opinion as to what is useful, which suggests to me that ICloneable may not be broken so much as incomplete. This brings about the idea of not obsoleting ICloneable, but instead supplementing it with IExplicitCloneable (with a better name, one would hope) which would derive from ICloneable and provide both generic cloning(that is class defined semantics) with the ICloneable::Clone method and contractually linked DeepClone and ShallowClone methods that have documented and expected behaviour. I think it would work together pretty well, semantically. It allows the class to specify generic cloning circumstances but still sets a standard where client code can control cloning behaviour. The downside however is it forces the class to choose a default to provide in Clone and to implement both deep and shallow cloning, Im’ not sure what kind of effects that would have. You could split the interfaces in two, as well, but having the generic ICloneable base has its ups.

    Another option would be to create a modified version of ICloneable(hopefuly not called ICloneable2) which takes an enum parameter specifying expected behaviour. I think I prefer the multiple method approach however.

  7. Luc Cluitmans says:

    The problem with ICloneable seems to me somewhat of a language issue. The idea behind ICloneable is that an objects tells you that it supports the ‘cloning’ design pattern: the class tells you (the developer using the class) that the object can be cloned, and gives the hint that it supports this design pattern. I leave out the deep/nondeep clone issue. The problem is: is using an interface the best way to convey this meaning. Probably it is, but only in the sense that alternatives are worse. I think that getting rid of it is the best.

    However, when discussing this one should also look at existing classes that don’t implement ICloneable, but do support the same semantics, and have a Clone method. The prime example of such class is probably XPathNavigator, and client code using that class very heavily depends on its clone method. Considering this, I realize there is another dimension than the deep/nondeep copy issue, namely the issue of efficiency. For some implementations, the efficiency of the implementation of Clone() doesn’t really matter. For others it does – If you trace the calls to Clone() in a custom subclass of XPathNavigator, when feeding that object to e.g. the XSLT engine, you will notice that Clone() is easily the most frequently called method, and obviously implementation efficiency is important.

  8. David Levine says:

    I’d say obsolete it – many other calls are already being obsoleted and the semantic uncertainty this creates is not worth the benefit. There already is enough confusion about the differences between deep and shallow cloning versus copying, and there doesn’t appear to be much benefit to implementing this interface.

    Classes are already free to implement a copy or a clone method, using the semantics that is most appropriate for it.

    I would not like to see an ICloneable2, 3, etc. If you can tighten up the definition to where the interface is usable then I’d prefer to see that. If not, then drop it and leave it up to the class designer to expose Copy methods.

  9. J. Daniel Smith says:

    I agree with Judah Himango: fix your "mistakes" now while there is relatively little pain. Things are only going to get worse as more people start using .NET.

  10. Ron says:

    I’d recommend waiting until a suitable replacement is available. If a replacement comes with the 2.0 version of the framework (or is planned to ship with 2.0 though not yet fully defined), then it’s ok to obsolete the interface.

    I agree with Frank. Each object has (or should have) its own idea of what is necessary to clone. It’s also quite possible the object has no need or intention of providing both a deep and shallow. Clearly a clone contract mechanism is required, but I don’t think it should be an all inclusive one (providing both a generic and specific clone methods). Thus, I disagree with defining a new interface and having it derive from the generic case.

    To me, the best option would be creating an IGenericClone (though I think the generic case could also just be called IClone) with a single Clone() method. A companion interface called IProperClone with a single Clone(CloneType cloneType) method where CloneType is defined as an Enum with the members Deep and Shallow.

  11. Darrell says:

    "Close to unusable" ? It *is* unusable, except in any code that was developed under your personal guidance (ie, on your team). Go ahead, obsolete it. It will still compile if people need it to. I don’t care if you have a replacement or not, stop the damage as quickly as possible, before it spreads like a virus.

  12. Rune Huseby says:

    Make IClonable obsolete and introduce IDeepClonable (or similar).

  13. Daniel, Ron: Let’s consider the "owning collection" first. This is a collection, or an object containing a collection, for which a shallow clone makes no sense. These are very common. People build object models with these types of collections and composites all the time. It is standard practice.

    These types of objects should not have any possible "shallow clone" method — it simply doesn’t make sense. Internally, they may need to do some lazy deep cloning, for performance reasons, but semantically, to the user, it always appears to be a "correct" clone.

    Consider a "reference collection", a collection of references shared objects. For these objects, a deep clone never makes sense. Again, all we need is a method that performs "correct" clone.

    Traditionally, in OOP, this is exactly what the word "clone" meant: do whatever is necessary to make a usable copy. For these objects, a generic "clone" is all that is needed, and anything more adds confusion.

    Now lets consider the other type of collection, such as an array list. This does not have any "owning" semantic, or "reference" semantic — it is simply a collection, and it is dumb. It has no idea what will be needed, a deep or shallow clone. For these objects, we need to distinguish between deep and shallow clone methods, definitely. And the user of the object must determine the best method to call.

    In an ideal world, I would prefer: keep ICloneable, but change the semantics to be "perform the correct clone for this class". For dumb, non-owning collections such as ArrayList, ICloneable.Clone should mean deep clone, simply because that is the most common.

    An additional interface, which I have no idea what to call, could be used on the dumb collections, with two methods, DeepClone and ShallowClone.

    I know that ICloneable may be "ruined" because its semantics were never specified and communicated precisely. In fact, the specification is not so important as the communication — if people don’t understand, any new interface will be similarly "ruined" in the future.

  14. Philip Rieck says:

    Obsolete it, but *please* include a replacement (like IDeepClonable or similar). With Generics at our doorstep, I’d like to make sure that this sematic contract can be specified in a generic type constraint. Simply obsoleting it and specifying guidelines for a pattern doesn’t help us on that front*.

    *Generics in net2.0 are heavily rooted in strong-typedness. Method signature "patterns" are more dynamic-typedness. The two don’t mix very well at the moment [CTP1]. For example, I can say "allow construction of this with any type <t> implementing IDeepClonable", but I can’t say "allow construction of this with any type <t> that I can call ‘public <t> Copy()’ on". Note that in either case, having the contract be "ridiculously" unclear makes them unusable.

  15. One possibility would be to obsolete not the interface, but the Clone() method, and replace it witha Clone(bool deepCopy) so at least you’d have to be explicit about what you wanted. The drawback would be implementers would have to allow for both.

  16. The problem is with the DOCUMENTATION of the Clone method ("Clone can be implemented EITHER as a deep copy or a shallow copy"…) non with the interface itself. Cloning has to be cloning! – always deep. I’m against obsoleting the interface but in favor of its deep interpretation

  17. Daniel O'Connell says:

    Frank: You are missing one point. If a simple collection is allowed to determine deep and shallow cloning, the contents of that collection must be capable of it. A deep clone must be supported down to every leaf of the graph, otherwise it is(probably) not possible. Deep and shallow clones need to be used and considered at much deeper levels. Also, I would argue that there are situations where shallow or deep clones could be used with owning collections, however I think that an owning collection will often be somewhere in the middle ground. The definition of ICloneable::Clone, as it stands, gives you basically what you want. However, the problem is when you call List<Customer>::DeepClone(assume Customer cannot be deeply cloned for consistency reasons). How do you deeply clone objects that refuse to participate in deep cloning? There is a tremendous semantic issue here…one which I fear we’d have to use can properties to determine i fa particular method is valid:

    public interface IExplicitlyCloneable

    {

    object DeepClone();

    object ShallowClone();

    bool CanDeepClone{get;}

    bool CanShallowClone{get;}

    }

    otherwise, there is no particular way for generic lists to handle the situation. I fear that if cloning isn’t standardized and required to support all choices, then you can never rely on any of them.

    As has been mentioned, ICLoneable works on your own code, but it is frightfully dangerous and troublesome on otehr code. Without extending cloning semantics to support and require thought about cloning and a way to publish its abilities, I don’t know that we’ll ever get anywhere.

    We could help get by this by exposing seperate interfaces, ICloneable, IDeepCloneable : ICloneable and IShallowCloneable : ICloneable which would allow publishing ability without properties, but it still leaves alot of guesswork in collections. For example, what is List<T>::DeepClone supposed to do if you add one object that only supports IShallowCloneable? It would either fail and throw an exception or it would have to just deal with that particular member as being non-cloned. This same issue exists with ICloneable, what do you do with an assumed deep copy of List<Customer>::Clone? It is very difficult to know what happens here…a deep copy isn’t possible because Customer can’t deep copy, but that is what is expected. You can’t even throw an exception, you just labor on assuming things worked when they didn’t.

    I think, in the end, the easiest sollution would to have List not support any form of cloning and let derived lists with constraints handle it. However, it does leave some nasty issues, and I’m not sure how to fix it all, or if it even can be fixed.

  18. Keith Patrick says:

    I literally tacked on ICloneable to about 5 of my classes a second ago! And I was sitting there thinking about the deep vs. shallow issue as well (I was implementing deep, as my classes had ICloneable children). My preference is:

    ICloneable.Clone(Boolean deep);

    There are only going to be two levels, unless you wanted to start making up arbitrary depths, but then you’ve got much bigger contract issues than shallow vs. deep.

  19. Daniel O'Connell says:

    For those of you suggesting adding a methodchanging the method signature of Clone, remember that you can’t do that. Doing so would break both binary and source compatibility and would make people unhappy. Unfortunatly a new interface with a new contract would have to be exposed, and unless a system such as I suggested(that is, adding explicit methods while leaving ICloneable up to the implementer) is put into place, I think ICloneable itself will have to be obsoleted.

    Personally, I think there are three effective levels: deep, shallow, and Implementation defined. With this scheme, anyone who chooses to use the implementation defined cloning is implicitly bound to pay attention to the implementations explicit contract, that would be part of the contract of the method. It is valid and perhaps nessecery in some cases(consider ones where all but a single object can be deeply cloned, but that one object must be single instance for security reasons).

    I don’t think there is anything wrong with ImplementationDefined cloning as long as it is 1) clearly defined as implementation defined, not shallow or deep as the current ICloneable::Clone is and 2) there are clear, seperatly defined methods for standard shallow and deep cloning. I prefer seperate interfaces so you don’t have to deal with the case of detecting whether a client doesn’t support shallow or deep or both. With multiple interfaces, its both clear to the caller that they can’t perform a deep clone and its constrainable with generics. Also, since you probably can’t require a class to support all forms of cloning, a boolean parameter doesn’t enforce that without adding Can properties like I mentioned in a previous comment nor does it allow you to define a deep or shallow clone constraint. Thus, the clients behaviour remains unpredictable and we end up in a slightly differently flawed situation, IMHO.

  20. Om says:

    hello

    why don’t you simply change the interface as this

    public interface IClonable {

    void Clone(bool deep);

    }

    for backward compatibily, hum

    #if LOOKBACK

    namespace System.Downgrade {

    interface IClonable() {

    void Clone();

    }

    }

    #endif

    namespace System {

    #if LOOKBACK

    public interface IClonable : System.Downgrade.IClonable {

    #else

    public interface IClonable {

    #endif

    void Clone(bool deep);

    }

    }

  21. Om says:

    a great feature will be to enhance refactory | msbuild | nant | msh | perl

    to update using clause in a discovery/registry contract mode (:

  22. Neil Groves says:

    Perhaps the interface is ok, and one could leverage the generic capabilities to provide a relevant cloning policy implementation. This might at least help with disambiguating desired cloning behaviour for the legacy collections.

    class DeepClonePolicy : ICloneable

    {

    pubilc static T Clone(T source)

    {

    return T.DeepClone(source);

    }

    }

    class ShallowClonePolicy : ICloneable

    {

    public static T Clone(T source)

    {

    return T.ShallowClone(source);

    }

    }

    class Program

    {

    static void main()

    {

    // Approach using ClonePolicy as second generic parameter.

    // The List implementation would be modified to all the static // method on the policy avoiding polymorphic call overhead

    // Hopefully this would be inlined away to zero overhead in

    // trivial cases, just like in C++.

    List<Foo,DeepClonePolicy> l = new List<Foo,DeepClonePolicy>();

    }

    }

    What is interesting is that this is an acknowledged failure to appropriately define the contract. Will we see an assertion mechanism like in Eiffel for Design-by-contract with appropriate configurable run-time level assertions? This is about the only thing I haven’t found in the Visual Studio .NET 2005.

    I have to say, the 2005 .NET is fantastic. A huge improvement on it’s predecessor. The implementation of the generics into the framework, the superb use of predicates, and the encapsulation of the iteration mechanisms into the core collections is superb. Is this due to Bertrand Meyer’s involvement?

  23. Neil Groves says:

    class DeepClonePolicy<T>

    {

    public static T Clone(T source)

    {

    return T.DeepClone(source);

    }

    }

    class ShallowClonePolicy<T>

    {

    public static T Clone(T source)

    {

    return T.ShallowClone(source);

    }

    }

    class Program

    {

    static void main()

    {

    // Approach using ClonePolicy as second generic parameter.

    // The List implementation would be modified to all the static

    // method on the policy avoiding polymorphic call overhead

    // Hopefully this would be inlined away to zero overhead in

    // trivial cases, just like in C++.

    List<Foo,DeepClonePolicy<Foo> > l = new List<Foo,DeepClonePolicy<Foo> >();

    }

    }

    As a further note this separation of the Cloning policy allows for other cloning behaviour than just shallow and deep. I’m sure plenty of experienced software engineers have faced problems where classes with deep associations require other levels of cloning, where some are shallow and some are deep, and indeed this cloning can vary between aspects of the architecture. Hence it is better to have separate cloning policies in these cases than attempt to build all of the possible cloning capabilities into the class.

    Apologies for the inocrrect submission earlier. I was a little slap happy with the Submit button.

  24. Mark Levison says:

    Please obselete ICloneable. If we’re forced to recompile and fix our apps in the next couple of years it won’t be a huge loss. I haven’t thought enough about replacements although Neil’s suggestion seems elegant.

  25. IMHO, ICloneable has two problems, neither of which is deep/shallow semantic vagueness. Whether a clone is deep, shallow, or somewhere in between *should* be an implementation detail.

    Consider MenuItem, in winforms. It maintains a ref to its parent Menu object. Should a "deep" copy of a MenuItem spider into a deep copy of the whole Menu object? Of course not.

    No, the semantics of ICloneable are necessarily vague — any given class in a non-trivial framework is likely going to have to do some deep *and* some shallow copying, in order to clone itself meaningfully.

    There are, however, two other problems with ICloneable — one technical, one philosophical.

    Technical: it’s very difficult for derived types to override and extend their base class’s ICloneable impl. http://www.windojitsu.com/blog/copyctorvsicloneable.html

    Philosophical: it’s not really a "system" interface, in that it’s never actually *used* by the system. Contrast with most of the other base system-defined interfaces (ISerializable, IComparable, IEnumerable, IDisposable, etc.) each of which is either called by the .NET runtime in some situation, or else serves to support some language feature (e.g.: C#’s foreach and using constructs are supported by emitting calls into IEnumerable and IDisposable).

    So. I say "yes", do obsolete ICloneable — let each type define its own "Clone" method and appropriate semantics. And let us go back to encouraging copy-constructors for non-sealed, copy-able types, so that derived classes may more easily extend their bases’ copy semantics, properly.

  26. AT says:

    IMHO, Best solution to IClonable problem must be some language/runtime improvement to decrease costs of deep cloning.

    Once you called clone – you have current version of object. All modifications using others handles do not affect you. But also your clone do not make a complete copy.

    I.e. Array with 10000 elements after clone and changing 10 or 20 elements must not occupy 20000 units of memory – but about 10020.

    Some kind of versioned objects will be good. Similar to versioned database rows, to get old version database rollback recent transactions or keep record of changes.

    Transactional programming language will be cool ;o))

  27. cody says:

    Defining framework interfaces or methods like DeepClone and

    ShallowClone is also unusable because there can be lots of possibilities which fields are acutally just copied or deep copied.

    The best way ist to leave this stuff out of the framework and let the users defined their own interfaces and methods for cloning.

  28. Kelly White says:

    As I&#39;ve mentioned earlier, I have a long commute to and from work each day. Each day as I ride the