Should I Implement ICloneable?

Question

I have written some codes and found that two classes (namely Fish and Mammal below) have a same pattern so I decided to sum up with generics.

The problem is, I need copy a constructor from the base class part.

Also, this can't be fixed using new() constraints (CS0304) since the constructors are not default (0-parameter) ones.

I write this because I was told implementing ICloneable is not a good practice.

public class OneHeart {...}
public class TwoHeart {...}

public class Animal<TyHeart> /* : ICloneable ?*/ {
    public Animal(TyHeart heart) {
        if(heart==null) throw new ArgumentNullException();
        Heart = heart;
    }
    public Heart { get; set; }
}
public class Fish : Animal<OneHeart> {
    public Fish(OneHeart heart) : base(heart) {}
    public Fish(Fish fish) : base(fish.Heart) {} // copy ctor but no use?
}
public class Mammal : Animal<TwoHeart> {
    public Mammal(TwoHeart heart, Organ lung) : base(heart) {Lungs=lung;}
    public Mammal(Mammal mammal) : base(mammal.Heart) {Lungs=mammal.Lung;}
    public Organ Lungs {get; set;} // Mammal-only member:)
}
// The Zoo collects animals of only one type:
public class Zoo<TyHeart, TyAnimal> : LinkedList<TyAnimal> 
    where TyAnimal : Animal<TyHeart> {
    public Zoo() : base() {}
    public Zoo(Zoo<TyHeart, TyAnimal> srcZoo) {
        foreach(var animal in srcZoo) {
            // CS0304 compile error:
            base.AddLast(new TyAnimal(animal));
        }
    }
    ...
}

Fish and Mammal are the only classes derived from Animal and

I know they both implement copy constructors.

EDIT: No deep-copy needed.

(Types of) Hearts and Lungs are singleton and shared among Animal/Fish/Mammal.

score 1 · Accepted Answer · answered Jun 26 '13 at 00:14

A problem with implementing ICloneable is that a storage location (field, variable, array slot, etc.) of a mutable reference type may be used to encapsulate identity, mutable state, both, or neither, but neither .NET nor any of its languages include any standard convention to indicate which of the above any given storage location is supposed to encapsulate.

If a field Foo1 encapsulates identity but not mutable state, then George.Clone().Foo1 should refer to the same instance as George.Foo1.

If a field Foo2 encapsulates mutable state but not identity, then George.Clone().Foo2 should refer to a new object which is initialized to have the same state as George.Foo2.

If a field Foo3 encapsulates neither mutable state nor identity [i.e. only immutable state other than identity], then George.Clone().Foo3 may refer to George.Foo3, a new object with the same state, or any other convenient object with the same state.

If a field Foo4 encapsulates both mutable state and identity, instances of the type cannot be meaningfully cloned using a Clone method.

If there were different storage location types for things that encapsulate identity, mutable state, both, or neither, there would be little for a deep- versus shallow distinction, since any reference-type field or other storage location encapsulated by a type should be cloned by recursively applying the above rules (note that for properly-constructed objects this cannot result in endless recursion, since two objects cannot meaningfully encapsulate each others' mutable state without at least one of them also encapsulating the other's identity). Unfortunately, because no such distinctions exist in the type system, there's no practical solution except to either implement ad-hoc cloning methods, or else implement a cloning method which uses attributes to decide what it should do, and uses hard-coded behavior for built-in .NET types which don't include any special indicators.

Zoran Horvat · Answer 2 · 2015-07-21T13:02:00.667

0

Default base constructor will be called anyway if not stated differently by giving the : base(...) at the derived constructor declaration.

You don't have to implement ICloneable since it may make you troubles regarding the question is it deep or is it shallow copy. But anyway, you can follow the cloning pattern (only call it differently).

Here is a resource that can lead you in implementing the cloning facility with or without the ICloneable interface: How to Implement ICloneable Interface in Derivable Classes in .NET.

The idea is to create a protected copy constructor aside from the default constructor. That would let you keep the new() constraint and still provide derived classes with option to clone base and derived content when required.

edited Jul 21 '15 at 13:02

answered Jun 25 '13 at 15:44

Zoran Horvat

10,924
3
31
43

Inheritable classes should not have cloning members which don't chain to `MemberwiseClone`. Otherwise, calling `Clone` on an instance of a derived class which doesn't override it will yield an instance of the wrong type. – supercat Jun 26 '13 at 00:05
@supercat Isn't that the case with all virtual methods? Forget about ICloneable interface for a moment and imagine you implemented your own cloning mechanism (e.g. Copy method). This method would naturally return reference to base type. Any inheritor would have to override it or it wouldn't work correctly. Relying on MemberwiseClone does not solve the problem of deep cloning. – Zoran Horvat Jul 17 '13 at 14:53
If cloning members form a chain which goes back to `MemberwiseClone` [preferably chained using a protected virtual method] and a derived class does not create any privately-held mutable objects of its own, the parent's cloning method will work correctly to create an instance of the derived type. Otherwise, *every* derived class will have to define a cloning-method override even if the only thing it does is chain to the parent's copy constructor, and failure to do so won't cause a compiler diagnostic--just broken behavior. – supercat Jul 17 '13 at 15:24
It I had my druthers, `MemberwiseClone` would never have been an instance member of `Object`, but instead a class `System.CloneableObject` would have had a protected virtual `CloneBase` method (whose base implementation would behave like `MemberwiseClone`). I consider it unfortunate that most objects expose a protected `MemberwiseClone` method, when there is in many cases no way a derived object could possibly use it correctly. – supercat Jul 17 '13 at 15:28

Should I Implement ICloneable?

2 Answers2