Why No Icloneable<T>

Why no ICloneableT?

ICloneable is considered a bad API now, since it does not specify whether the result is a deep or a shallow copy. I think this is why they do not improve this interface.

You can probably do a typed cloning extension method, but I think it would require a different name since extension methods have less priority than original ones.

How to use ICloneableT when T is ListT?

You can't do this, because you can't define List<T> yourself. You would only be able to do this if you could declare your own List<T> because of the way you've constrained ICloneable<T>. Since List<T> truly doesn't implement ICloneable<T>, you're going to have to have the type of T be InstanceList instead, which you do have control over.

Here's how you would implement it:

public class InstanceList : List<Instance>, ICloneable<InstanceList>
{
    public InstanceList Clone()
    {
        // Implement cloning guts here.
    }

    object ICloneable.Clone()
    {
        return ((ICloneable<InstanceList>) this).Clone();
    }
}

public class Instance
{

}

public interface ICloneable<T> : ICloneable where T : ICloneable<T>
{
    new T Clone();
}

Of course, there is another alternative you could do. You could widen your generics a little bit, to create a CloneableList<T> type:

public class CloneableList<T> : List<T>, ICloneable<CloneableList<T>>
{
    public CloneableList<T> Clone()
    {
        throw new InvalidOperationException();
    }

    object ICloneable.Clone()
    {
        return ((ICloneable<CloneableList<T>>) this).Clone();
    }
}

public interface ICloneable<T> : ICloneable where T : ICloneable<T>
{
    new T Clone();
}

And if you really want to get fancy, create something that restricts T to ICloneable. Then you could implement ICloneable on the Instance class, and anything else you want to include in an ICloneable<T> list, thus treating every CloneableList<T> in the exact same way, avoiding a different implementation of ICloneable<T> for each and every cloneable list you want to create.

public class CloneableList<T> : List<T>, ICloneable<CloneableList<T>> where T : ICloneable
{
    public CloneableList<T> Clone()
    {
        var result = new CloneableList<T>();
        result.AddRange(this.Select(item => (T) item.Clone()));
        return result;
    }

    object ICloneable.Clone()
    {
        return ((ICloneable<CloneableList<T>>) this).Clone();
    }
}

public interface ICloneable<T> : ICloneable where T : ICloneable<T>
{
    new T Clone();
}

Why should I implement ICloneable in c#?

You shouldn't. Microsoft recommends against implementing ICloneable because there's no clear indication from the interface whether your Clone method performs a "deep" or "shallow" clone.

See this blog post from Brad Abrams back in 2003(!) for more information.

Why is `ListT` not clonable?

The problem with cloning objects and, especially the ICloneable interface, is that the public interface doesn't communicate the intention well.

Namely - will such Clone function of the List<T> clone contained elements as well, or just clone the list and copy the references to contained elements? Shallow copy, which copies the references and only creates the new list would be equivalent to this:

List<T> clone = new List<T>(originalList);

However, if you wanted to force all the contained elements to be cloned as well, then it would be equivalent to this:

List<T> clone = originalList.Select(x => (T)x.Clone()).ToList();

This assumes that the type T is implementing ICloneable. However, even with this solution, exact effects of code execution cannot be told in advance. What does it mean for an element x to clone itself? Will it be a shallow copy (offered by the MemberwiseClone method it inherits form System.Object), or will it be a deep copy. And if deep, what will happen if two objects in the list are referencing the same third object? Will that third object be copied twice or only once? And so on... you can see where this is going.

For all the reasons listed above, cloning facilities are not incorporated in the framework. It is left to custom code to decide what it means to clone an object and then implement custom code for that.

How to workaround missing ICloneable interface when porting .NET library to PCL?

ICloneable is kind of the textbook example of where we made a mistake when designing an API. Whether it does a deep or shallow copy is undefined, and the Framework Design Guidelines now recommend not using it.

That said, if you need to use it anyway, you can probably do so from PCLs by defining the interface (with the exact same name) in a PCL, and when you are running on a platform that does have ICloneable, replacing the assembly with the ICloneable definition with one which has a type forward to the "real" version. See my answer here for a bit more on this: Is there any way I can implement IValidatableObject on Portable Class Library Project?

Resharper, ICloneable and never null

ReSharper assumes that your object adheres to the contract of ICloneable, which says among other things that

The resulting clone must be of the same type as, or compatible with, the original instance.

From the fact that data is checked to be non-null and the assumption that you return an object of the same or compatible type from your implementation of ICloneable.Clone() ReSharper concludes that copy is also non-null, triggering the warning.

Of course it is definitely possible to return null from Clone. However, returning null would be a coding error, so it is a good idea to skip that null check.

ICloneable Interface

  clonedCloner.MyContent = MyContent.Clone();

The Content class does not implement ICloneable so this statement cannot compile. You are getting a bit lost, I can not see which implementation of Cloner you actually used and how this class has anything to do with Person.

Do note that you did not actually test whether you got a deep clone. You only tested for the shallow clone case by checking that the collection wasn't being modified. A deep clone test would, say, alter a property of Mick and check that the original collection still has an unmodified Mick. Which is really the meaning of "deep". Which is okay in your code, you however lose elegance points for using MemberwiseClone(), it doesn't do anything useful and is the opposite of deep cloning.

Frankly, the book isn't teaching you very good practices. The ICloneable interface narrowly escaped being deprecated and should be avoided. It is a broken interface, it doesn't allow the caller to specify whether a deep or a shallow copy is desired. Too often, it is implemented as a shallow copy, because it is cheap and easy, while the caller really wanted a deep copy. Producing a nasty bug that is pretty hard to diagnose.

If you want to support deep-cloning then just add a Person DeepClone() method to your class. Now it is explicit and type-safe.

Don't dwell on this, move on in the book. Do consider finding a better one.

How to implement ICloneable without inviting future object-slicing

It's a while ago that I faced the very same issue in the very same situation without finding a satisfying solution.

Thinking about this again, I found something which might be a solution (at best):

#include <iostream>
#include <typeinfo>
#include <typeindex>

class Base { // abstract
  protected:
    Base() = default;
    Base(const Base&) = default;
    Base& operator=(const Base&) = default;
  public:
    virtual ~Base() = default;
    
    Base* clone() const
    {
      Base *pClone = this->onClone();
      const std::type_info &tiClone = typeid(*pClone);
      const std::type_info &tiThis = typeid(*this);
#if 0 // in production
      assert(std::type_index(tiClone) == type_index(tiThis)
        && "Missing overload of onClone()!");
#else // for demo
      if (std::type_index(tiClone) != std::type_index(tiThis)) {
        std::cout << "ERROR: Missing overload of onClone()!\n"
          << "  in " << tiThis.name() << '\n';
      }
#endif // 0
      return pClone;
    }
    
  protected:
    virtual Base* onClone() const = 0;
};

class Instanceable: public Base {
  public:
    Instanceable() = default;
    Instanceable(const Instanceable&) = default;
    Instanceable& operator=(const Instanceable&) = default;
    virtual ~Instanceable() = default;
    
  protected:
    virtual Base* onClone() const { return new Instanceable(*this); }
};

class Derived: public Instanceable {
  public:
    Derived() = default;
    Derived(const Derived&) = default;
    Derived& operator=(const Derived&) = default;
    virtual ~Derived() = default;
    
  protected:
    virtual Base* onClone() const override { return new Derived(*this); }
};

class WrongDerived: public Derived {
  public:
    WrongDerived() = default;
    WrongDerived(const WrongDerived&) = default;
    WrongDerived& operator=(const WrongDerived&) = default;
    virtual ~WrongDerived() = default;

  // override missing
};

class BadDerived: public Derived {
  public:
    BadDerived() = default;
    BadDerived(const BadDerived&) = default;
    BadDerived& operator=(const BadDerived&) = default;
    virtual ~BadDerived() = default;

  // copy/paste error
  protected:
    virtual Base* onClone() const override { return new Derived(*this); }
};

#define DEBUG(...) std::cout << #__VA_ARGS__ << ";\n"; __VA_ARGS__ 

int main()
{
  DEBUG(Instanceable obj1);
  DEBUG(Base *pObj1Clone = obj1.clone());
  DEBUG(std::cout << "-> " << typeid(*pObj1Clone).name() << "\n\n");
  DEBUG(Derived obj2);
  DEBUG(Base *pObj2Clone = obj2.clone());
  DEBUG(std::cout << "-> " << typeid(*pObj2Clone).name() << "\n\n");
  DEBUG(WrongDerived obj3);
  DEBUG(Base *pObj3Clone = obj3.clone());
  DEBUG(std::cout << "-> " << typeid(*pObj3Clone).name() << "\n\n");
  DEBUG(BadDerived obj4);
  DEBUG(Base *pObj4Clone = obj4.clone());
  DEBUG(std::cout << "-> " << typeid(*pObj4Clone).name() << "\n\n");
}

Output:

Instanceable obj1;
Base *pObj1Clone = obj1.clone();
std::cout << "-> " << typeid(*pObj1Clone).name() << '\n';
-> 12Instanceable

Derived obj2;
Base *pObj2Clone = obj2.clone();
std::cout << "-> " << typeid(*pObj2Clone).name() << '\n';
-> 7Derived

WrongDerived obj3;
Base *pObj3Clone = obj3.clone();
ERROR: Missing overload of onClone()!
  in 12WrongDerived
std::cout << "-> " << typeid(*pObj3Clone).name() << '\n';
-> 7Derived

BadDerived obj4;
Base *pObj4Clone = obj4.clone();
ERROR: Missing overload of onClone()!
  in 10BadDerived
std::cout << "-> " << typeid(*pObj4Clone).name() << '\n';
-> 7Derived

Live Demo on coliru

The trick is actually quite easy:

Instead of overriding clone() itself, it is used as trampoline into a virtual onClone() method. Hence, clone() can check the result for correctness before returning it.

This is not a compile-time check but a run-time check (what I consider as second best option). Assuming that every class of a class library in development should hopefully be checked / debugged at least during development I find this quite reliable.

---

The accepted answer to SO: Reusable member function in C++ showed me a way to make this even more immune against copy/paste errors:

Instead of typing out the class name in every overridden clone(), the class name is obtained via decltype():

class Instanceable: public Base {
  public:
    Instanceable() = default;
    Instanceable(const Instanceable&) = default;
    Instanceable& operator=(const Instanceable&) = default;
    virtual ~Instanceable() = default;
    
  protected:
    virtual Base* onClone() const
    {
      return new std::remove_const_t<std::remove_pointer_t<decltype(this)>>(*this);
    }
};

class Derived: public Instanceable {
  public:
    Derived() = default;
    Derived(const Derived&) = default;
    Derived& operator=(const Derived&) = default;
    virtual ~Derived() = default;
    
  protected:
    virtual Base* onClone() const override
    {
      return new std::remove_const_t<std::remove_pointer_t<decltype(this)>>(*this);
    }
};

Live Demo on coliru

---

Related Topics