Consider the reference Josh Smith' article WPF Apps With The Model-View-ViewModel Design Pattern, specifically the example implementation of a RelayCommand
(In Figure 3). (No need to read through the entire article for this question.)
In general, I think the implementation is excellent, but I have a question about the delegation of CanExecuteChanged
subscriptions to the CommandManager
's RequerySuggested
event. The documentation for RequerySuggested
states:
Since this event is static, it will only hold onto the handler as a weak reference. Objects that listen for this event should keep a strong reference to their event handler to avoid it being garbage collected. This can be accomplished by having a private field and assigning the handler as the value before or after attaching to this event.
Yet the sample implementation of RelayCommand
does not maintain any such to the subscribed handler:
public event EventHandler CanExecuteChanged
{
add { CommandManager.RequerySuggested += value; }
remove { CommandManager.RequerySuggested -= value; }
}
RelayCommand
's client, requiring that the user of the RelayCommand
understand the implementation of CanExecuteChanged
and maintain a live reference themselves?If so, does it make sense to, e.g., modify the implementation of RelayCommand
to be something like the following to mitigate the potential premature GC of the CanExecuteChanged
subscriber:
// This event never actually fires. It's purely lifetime mgm't.
private event EventHandler canExecChangedRef;
public event EventHandler CanExecuteChanged
{
add
{
CommandManager.RequerySuggested += value;
this.canExecChangedRef += value;
}
remove
{
this.canExecChangedRef -= value;
CommandManager.RequerySuggested -= value;
}
}
I've found the answer in Josh's comment on his "Understanding Routed Commands" article:
[...] you have to use the WeakEvent pattern in your CanExecuteChanged event. This is because visual elements will hook that event, and since the command object might never be garbage collected until the app shuts down, there is a very real potential for a memory leak. [...]
The argument seems to be that CanExecuteChanged
implementors must only hold weakly to the registered handlers, since WPF Visuals
are to stupid to unhook themselves. This is most easily implemented by delegating to the CommandManager
, who already does this. Presumably for the same reason.