5
votes

Following this site: http://www.csharp411.com/c-object-clone-wars/

I decided to manually create a deep copy of my class (following site 1. Clone Manually). I implemented the clone interface and any necessary properties. I executed my program and checked if my clone was indeed equal too the original instance. This was correct.

However, my new instance still referenced to the original one. So any changes in my copy where reflected into the original instance.

So if this doesn't create a deep copy, then what does? What could have gone wrong?

(I want to make a deep copy manually to increase my performance, so I do not want to use the ObjectCopier class. (even if it works great, it takes 90% of my code run time).

Code Snippets:

Class implements:

public class SudokuAlgorithmNorvig: ICloneable
{

Clone method:

    public object Clone()
    {
        SudokuAlgorithmNorvig sudokuClone = new SudokuAlgorithmNorvig(this.BlockRows, this.BlockColumns);

        sudokuClone.IsSucces = this.IsSucces;

        if (this.Grid != null) sudokuClone.Grid = (Field[,])this.Grid;
        if (this.Peers != null) sudokuClone.Peers = (Hashtable)this.Peers;
        if (this.Units != null) sudokuClone.Units = (Hashtable)this.Units;

        return sudokuClone;
    }

Clone method call:

SudokuAlgorithmNorvig sudokuCopy = (SudokuAlgorithmNorvig)sudoku.Clone()

I did the same (implementing and setting clone method) in all my other classes. (Field + Coordinate)

1
We kind-of need to see your code to see what went wrong. The shortest code sample that exhibits the problem would be best.Matthew Watson
Yes, realized that :) Implement it now, thanksdylanmensaert
Ok, it looks like you're only doing a shallow clone of the object. For example, sudokuClone.Grid = (Field[,])this.Grid is NOT pointing sudokuClone.Grid at a new copy!Matthew Watson
I'm not sure since it C#, but if it is like Java since you assign all Objects (Field, Peers, Units) to the clone, they are passed by reference. You need the new operator to create the deep copy for each, that's why most objects have a constructor with parameter of type themselves.SGM1
@SGM that's why must objects have a constructor with parameter of type themselves N/A to c#I4V

1 Answers

3
votes

It looks like you're creating references to existing objects all over the place instead of creating copies.

Are BlockRows and BlockColumns custom objects that you're passing into the new object? Those will just be references to BlockRows and BlockColumns in the existing object, so changing one of those instances in the first object will be reflected in the second.

I don't know what Grid, Peers, and Units represent, but those will most likely be references too. You need to make all those classes cloneable as well. Otherwise, changing Grid in the first instance of your SudokuAlgorithmNorvig class will change the corresponding Grid in the second instance.