C# Cloning an ICloneable object containing List

Question

I have an issue while using my ICloneable class when it comes to cloning lists:

class MetroBoards : ICloneable
{
    public int Iteration;
    public List<int[]> MetroPieces = new List<int[]>();
    public List<char> MetroPiecesDefinition = new List<char>();

    public object Clone()
    {
        return this.MemberwiseClone();
    }
}

While trying to update the list in the clone made both this way:

MetroBoards NewBoard = new MetroBoards();
NewBoard = (MetroBoards)ChosenBoard.Clone();
NewBoard.MetroPieces[MoveOne] = Coordinates;

And this way:

MetroBoards (MetroBoards)ChosenBoard.Clone();
NewBoard.MetroPieces[MoveOne] = Coordinates;

Where MoveOne is an integer, and Cordinates are int[]. When I update the List in my clone it updates both the clone and the base.

As far as I have understood, the "MemberwiseClone" should provide me with a deep copy which should create a new object with copies of its properties, which shouldn't be associated with the new object.

EDIT

Thanks to comments I have re-read the documentation and modified the cloning process as:

class MetroBoards : ICloneable
{
    public int Iteration;
    public List<int[]> MetroPieces = new List<int[]>();
    public List<char> MetroPiecesDefinition = new List<char>();

    public object Clone()
    {
        MetroBoards ThisBoard = (MetroBoards)this.MemberwiseClone();
        ThisBoard.MetroPieces = new List<int[]>();
        foreach (int[] Piece in this.MetroPieces)
        {
            int[] temp = new int[2];
            temp[0] = Piece[0];
            temp[1] = Piece[1];
            ThisBoard.MetroPieces.Add(temp);
        }
        return ThisBoard;
    }
}

Thank you for making me understand my mistake.

MemberwiseClone only does a shallow copy. It's [documented](https://learn.microsoft.com/en-us/dotnet/api/system.object.memberwiseclone) this way — NineBerry, Sep 22 '18 at 14:23
ICloneable is unusable, it missed being deprecated by a hair on its chinny-chin-chin. Ultimately it couldn't, COM iterators require it. The interface does not have a good enough contract. Get rid of it and add a DeepCopy() method to your class. If you absolutely need an interface then declare your own, using DeepCopy instead of Clone so the contract is unambiguous. — Hans Passant, Sep 22 '18 at 16:00

score 0 · Answer 1 · answered Sep 22 '18 at 14:23

Have a look into the documentation:

The MemberwiseClone method creates a shallow copy by creating a new object, and then copying the nonstatic fields of the current object to the new object. If a field is a value type, a bit-by-bit copy of the field is performed. If a field is a reference type, the reference is copied but the referred object is not; therefore, the original object and its clone refer to the same object.

So, MemberwiseClone does not create a deep copy of the original object, leaving the original and the cloned object referencing the same list.

C# Cloning an ICloneable object containing List

1 Answers1