6

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)

Community
  • 1
  • 1
dylanmensaert
  • 1,689
  • 5
  • 24
  • 39
  • 2
    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 Mar 18 '13 at 22:30
  • 1
    Yes, realized that :) Implement it now, thanks – dylanmensaert Mar 18 '13 at 22:31
  • 1
    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 Mar 18 '13 at 22:33
  • 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 Mar 18 '13 at 22:35
  • @SGM `that's why must objects have a constructor with parameter of type themselves` N/A to c# – I4V Mar 18 '13 at 22:37

1 Answers1

3

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.

Grant Winney
  • 65,241
  • 13
  • 115
  • 165
  • Well, blockrows and blockcolumns are being set in the constructor of of SudokuAlgorithmNorvig. So don't really know how to clone that. – dylanmensaert Mar 18 '13 at 22:35
  • Grid is a 2dimensional array of type Field (custom class where I implemented clone in same way). Peers and Units are hashmaps which also consists of Field. So do I have to do something special with hashmap and array to be able to clone? – dylanmensaert Mar 18 '13 at 22:37
  • If it's like Java, I believe all primitives, int, double, char, etc. are passed by value – SGM1 Mar 18 '13 at 22:40