Apr 27, 2012 at 7:17 AM
Edited Apr 29, 2012 at 3:39 AM
|
Hi Laurent.
I was trying to understand the reasoning behind using WeakActions in RelayToCommand.
Does this try to target memory leak scenarios like the EventToCommand one?
If so then i have to disagree with the solution for the following reasons:
1) Commands ARE supposed to hold on to the ViewModel.
Its just the natural way of things.There is no point in weakly connecting them.If one goes away
then the other will as well.There is no point in the Command outliving its container ViewModel.
Its like adding a UserControl eventhandler on some control inside its visual tree. There is no
leak there because they both hold on to each other ,as they are meant to do, and they die
together.
If the fear is that anyone that keeps the Command also keeps the ViewModel then that is
reasonable and expected as well. If i need the command then i obviously need the ViewModel it
acts upon.
The Command anyways will be held through a databinding (DP) and there's built in support to be held weakly.
2) It forces a totally unnecessary restriction on the user of the API. Why o why should my already
encapsulated command logic be forced to be public or static??
This is the most trivial correct way of using commands from its inception :
public ICommand AddCommand
{
get { return new DelegateCommand(this.CanAdd, this.Add); }
}
private bool CanAdd() { return true; }
private void Add() {}
Does that mean that my private methods now HAVE to be public? If yes then *why*?
3) In trying to solve memory leaks it unfortunately focuses on the wrong link in the chain.
In the case of EventToCommand the problem is in the fact that "the Command holds on
to the EventToCommand" when it shouldn't.
That connection is the right one to be solved and made weak with a WeakEventListener.
So to recap :
- Commands are meant to strongly reference their ViewModel.
- Commands should not reference elements strongly when there is a reason.Its the responsibility of
the user of the Command (and not the Command itself) though to know that reason and provide an ad-hoc
solution like WeakEventListener (see EventToCommand)
- Command callbacks should be instance private if they want!
Looking forward to your thoughts on this.
Thanx
I have blogged about the issue here
|