1

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

JoanComasFdz
  • 2,911
  • 5
  • 34
  • 50
  • *"VM3 has a propertry that depends on VM2"* => it sounds like this is where the problems start. Everything in the question is quite abstract, but this sounds wrong. Can't you refactor so that all VMs depend only on their models? – Jon Nov 12 '13 at 15:22
  • Well let's say that the VM2.IsEnabled will modify VM3.IsVisible. I accept that kind of dependency. – JoanComasFdz Nov 13 '13 at 08:31

2 Answers2

1

In short, I would prefer to use a delegate to notify between view models. If you have a parent view model which has access to the various child view models (it seems that you do), then you can use one or more delegates like events to notify each other when updates are required.

Rather than typing out the whole scenario again, I'd prefer to point you towards my answer to the Passing parameters between viewmodels question, which provides a full description and code examples of this technique.

There is also a further addition that may interest you that can be found in my answer to the If necessary, how to call functions in MainViewViewModel from other ViewModels question. Please let me know if you have any questions.

Community
  • 1
  • 1
Sheridan
  • 68,826
  • 24
  • 143
  • 183
  • Basically you propose to stick to an event-based, specialized solution where you notify exactly what happened instead of using the generic "PorpertyChanged" event, right? – JoanComasFdz Nov 13 '13 at 08:36
  • I tend to prefer using `delegate`s because I can define whichever parameters suit the current situation best. However, I wouldn't think that your current scenario would give you a memory leak because it appears that your `VM2` and `VM3` objects are always alive anyway. Memory leaks only occur when an object that is no longer needed is kept alive by a hard reference to another object. – Sheridan Nov 13 '13 at 08:58
  • Well I am quite against using delegates directly. Altough an Action or Func are delegates, their usage is more clear to me since requires less code. – JoanComasFdz Nov 13 '13 at 14:19
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.

Mike Strobel
  • 25,075
  • 57
  • 69