Yellow
Yellow

Reputation: 3957

Event delegates disappear after being added in a foreach loop

I am trying to add event delegates to the objects I have got in a list. In the following example, I want to add the dinosaur_Jumped delegate whenever an object fires the associated event. I am adding them in a foreach loop, but somehow they disappear straight after that.

class MyViewModel
{
    MyViewModel(List<Dinosaur> dinosaurs)
    {
        // This works and creates the ViewModel the way I expect it to:
        m_dinosaurs = dinosaurs.Select( x => new DinosaurViewModel(x) );

        foreach (DinosaurViewModel dino in m_dinosaurs)
        {
            // This works within the scope of the loop
            dino.Jumped += dinosaur_Jumped;
        }

        // But now all my Jumped delegates are suddenly all gone
    }

    void dinosaur_Jumped(object sender, JumpingEventArgs e)
    {
        // This never gets called, even when the events do fire:
        Console.WriteLine("A dinosaur jumped");
    }

    private IEnumerable<DinosaurViewModel> m_dinosaurs;
}

I am presuming it has something to do with incorrect scope/closures or something; adding a delegate to a variable (in this case dino) that goes out of scope immediately, but I don't know how else I could this. Why doesn't this work?

Upvotes: 0

Views: 96

Answers (2)

Lukazoid
Lukazoid

Reputation: 19426

As I cannot see how you are checking your Jumped delegates, I will assume you are performing a subsequent iteration of m_dinosaurs.

Because Select is lazy, any subsequent iterations will result in different DinosaurViewModel instances being created, this means you are inspecting different instances to those which have had the event handler added.

A solution to this is to materialize the collection, e.g.

m_dinosaurs = dinosaurs.Select( x => new DinosaurViewModel(x) ).ToList();

Possible Garbage Collection

A less likely but possible situation results from the garbage collector, your Select statement will create a new DinosaurViewModel for each iteration, when you iterate m_dinosaurs and add the event handler the newly created DinosaurViewModel becomes eligible for garbage collection as there is nothing keeping a reference to it.

The solution is to ensure you keep a reference to each created DinosaurViewModel, the same solution as above will be sufficient as the .ToList() call will ensure a reference to each created DinosaurViewModel is retained, this means they are no longer eligible for garbage collection.

Upvotes: 3

user743382
user743382

Reputation:

Enumerable.Select is lazy. Very lazy. So lazy, in fact, that it completely disregards any output you've already seen from it. When you iterate over m_dinosaurs a second time, you get a completely fresh batch of DinosaurModel objects.

You may use dinosaurs.ConvertAll( x => new DinosaurViewModel(x) ) instead, to store the models in a list.

Upvotes: 2

Related Questions