Weird behavior of event handlers when deep cloning an object

Question

First off, sorry for not being able to come up with a better title. I tried, though.

I have a class like this

public class GameBoard : ICloneable {       
    private bool _isCloned;

    public ObservableCollection<BoardPosition> BoardPositions { get; set; }

    public GameBoard() {
        BoardPositions = new ObservableCollection<BoardPosition>();
        //this.BoardPositions.CollectionChanged += BoardPositionsOnCollectionChanged;
    }

    private void BoardPositionsOnCollectionChanged(object sender, NotifyCollectionChangedEventArgs notifyCollectionChangedEventArgs) {
        // if this event is raised from the NEW object, [this] still points to the OLD object
    }

    public object Clone() {
        //var gameBoard = (GameBoard)this.MemberwiseClone();
        var gameBoard = new GameBoard {
            // make it VERY clear this is just a clone
            _isCloned = true,
            // deep copy the list of BoardPositions
            BoardPositions =
                new ObservableCollection<BoardPosition>(this.BoardPositions.Select(p => p.Clone()).Cast<BoardPosition>())
        };

        gameBoard.BoardPositions.CollectionChanged += BoardPositionsOnCollectionChanged;
        // why does the target of the event point to the old object?!?
        return gameBoard;
    }
}
public class BoardPosition : ICloneable {
    public int[] Coordinates { get; set; }

    public BoardPosition(int[] coordinates) {
        Coordinates = coordinates;
    }

    public object Clone() {
        return new BoardPosition(new int[]{this.Coordinates[0], this.Coordinates[1], this.Coordinates[2]});
    }
}

which implements ICloneable. Inside the Clone method i deep-copy the ObservableCollection, and attach the CollectionChanged event handler.

Now the problem is, when the CollectionChanged event of the new, cloned object fires, [this] points to the OLD object that was cloned. This can easily be observed as _isCLoned is always false when the event is raised, even though it is set to true during cloning. Why is this, and what can i do about it? I want [this] to have a reference to the new, cloned object of course.

var gameBoard = new GameBoard();
        gameBoard.BoardPositions.Add(new BoardPosition(new[] {1, 2, 3}));

        var clonedBoard = (GameBoard)gameBoard.Clone();
        clonedBoard.BoardPositions.Add(new BoardPosition(new[] { 2, 3, 4 }));

score 4 · Accepted Answer · answered Feb 28 '13 at 08:02

4

That's because you don't attach the BoardPositionsOnCollectionChanged method of the newly created object but said method on the object being cloned.

That said, you need to specify the reference to use explicitly, or the compiler will assume this, which is the original object:

// your code ("this." is added implicitly):
gameBoard.BoardPositions.CollectionChanged += this.BoardPositionsOnCollectionChanged;

// working code:
gameBoard.BoardPositions.CollectionChanged += gameBoard.BoardPositionsOnCollectionChanged;

answered Feb 28 '13 at 08:02

Nuffin

3,882
18
34

1

good point on naming the event handler explicitly! I had not thought of that. – Lorenzo Dematté Feb 28 '13 at 08:04
I banged my head for hours trying to figure this out. Thank you so much! – lightxx Feb 28 '13 at 08:07

score 0 · Answer 2 · answered Feb 28 '13 at 08:00

why does the target of the event point to the old object?

Because you are assigning the handler inside the old object Clone method. I'm doing some guesswork here (I do not have a C# compiler here), but I remember a similar issue I had in the past. In Clone, "this" is the old object. BoardPositionsOnCollectionChanged resolves to the current "this" reference at that point, therefore it will always deal with a reference to the "old" object.

You may try factoring the assignement in a different "Init" method, and call it on the new instance:

void Init() {
   BoardPositions.CollectionChanged += BoardPositionsOnCollectionChanged;
}


public object Clone() {
    ...
    gameBoard.Init()
    return gameBoard;
}

that should work!

score 0 · Answer 3 · answered Feb 28 '13 at 08:02

Your problem is this line:

gameBoard.BoardPositions.CollectionChanged += BoardPositionsOnCollectionChanged;

BoardPositionsOnCollectionChanged is a member of the object being cloned, so when called, 'this' will be the old object.

Set up your event handler in the ctor instead.

Weird behavior of event handlers when deep cloning an object

3 Answers3