Implementing the ICloneable Interface C#(Question on Deep Cloning)

Question

Hello,

I was looking at code that implemented the ICloneable interface for one of the classes.

The class was as follows:

public class TempClass
{
  String[] names;
  String[] values;
}

A partial class was created that implemented TempClass

public partial class TempClass:ICloneable
{
   public Object Clone()
   {
      TempClass cloneClass = new TempClass();
      String[] cloneNames = new String[this.names.Length - 1];
      String[] cloneValues = new String[this.values.Length -1];

      Array.Copy(this.names,cloneNames,this.names.Length);
      Array.Copy(this.values,cloneValues,this.values.Length);

      cloneClass.names = cloneNames;
      cloneValues.values = cloneValues;

      return cloneClass;

   }

}

I was wondering if this would be a valid way of doing a deep copy of an object? What raises flags here is the intermediate structures cloneNames and cloneValues that are used to copy the values of the original object and have the member variables Names and Values point to it and then return the object reference that was created in the clone method.

Any feedback on this snippet will be well appreciated

Thanks

Answer 1

Well, there's one problem - you're not creating the new string arrays of the right size. It should be:

String[] cloneNames = new String[this.names.Length];
String[] cloneValues = new String[this.values.Length];

Or as a simpler solution:

String[] cloneNames = (String[]) this.names.Clone();
String[] cloneValues = (String[]) this.values.Clone();

Other than that, it should be okay. Could you explain your concern in more detail?

Answer 2

Array.Clone and Array.Copy both create shallow copies (as described in the linked API documentation), but using string arrays clouds the issue a bit because strings are immutable in .NET. When you do this:

stringArray[0] = "first value";
stringArray[0] = "second value";

The first string instance is replaced by a different one, so it looks like you've made a deep copy (and for string arrays, you might as well have done just that).

To perform a deep copy of an object with arrays of normal reference types, you need to create new instances of the array elements. Say TempClass stored an array of Cookie objects instead of strings:

public Object Clone() {
    TempClass clone = new TempClass();

    clone.cookieArray = new Cookie[this.cookieArray.Length];
    for(int i = 0; i < this.cookieArray.Length; i++) {
        Cookie cookie  = this.cookieArray[i];
        clone.cookieArray[i] = new Cookie(cookie.Name, cookie.Value, cookie.Path, cookie.Domain);
    }
}

Answer 3

You can rewrite your clone method as follows, otherwise pony is right.

public Object Clone()
{
    return new TempClass
    {
        names = this.names == null? null:this.names.ToArray(),
        values = this.values == null? null:this.values.ToArray()
    };
}

It may be slightly slower than Clone, but it works.