Pass Action instead of hooking PropertyChanged?

Question

In my current project I've faced the following situation:

• VM1 is used to be shown on a screen.
• VM1 has a public property of VM2.
• VM1 has a public property of VM3.
• VM3 has a propertry that depends on VM2.
• VM1 has no disposing mechanism.

At the beginning I thought about hooking to VM2.PropertyChanged event to check for the property I want and change the VM3 affected property accordingly, as:

public class VM1 : INotifyPropertyChanged
{
    public property VM2 VM2 { get; private set; }
    public property VM3 VM3 { get; private set; }

    public VM1()
    {
        this.VM2 = new VM2();
        this.VM3 = new VM3();
        this.VM2.PropertyChanged += this.VM2_PropertyChanged;
    }

    private void VM2_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        // if e.PropertyName equals bla then VM3.SomeProperty = lol.     
    }
}

This means that, since I can not unhook the event in this class, I have a memory leak.

So I end up passing an Action to VM2 so that it will be called when its important property changes the value, as:

public class VM2 : INotifyPropertyChanged
{
    public Action WhenP1Changes { get; set; }

    private bool _p1;
    public bool P1
    {
        get
        {
            return _p1;
        }
        set
        {
            _p1 = value;
            this.WhenP1Changes();
            this.PropertyChanged(this, new PropertChangedEventArgs("P1");
        }
    }
}

public class VM1 : INotifyPropertyChanged
{
    public VM2 VM2 { get; private set; }
    public VM3 VM3 { get; private set; }

    public VM1()
    {
        this.VM2 = new VM2();
        this.VM3 = new VM3();
        this.VM2.WhenP1Changes = () => VM3.SomeProperty = "lol";
    }
}

Do I have a memory leak here?

PD: It would be great if you can also answer to: - Is this even a good practice?

Thanks

Answer 1

Do I have a memory leak here?

The lambda assigned to VM2.WhenP1Changes captures this VM1 instance (needed to access the VM3 property), so as long as the view model VM2 is alive, it will keep VM1 alive. Whether this ends up being a leak depends on the lifecycle of those view models, but the implications are effectively the same as your first example using events.