-1

Let's say you want to create an ICommand that can always be executed. Then it's canExecute method should always return true. And the CanExecuteChanged event is basically not very useful. In my mind it would be bad to allow subscribing to an event you know will never be fired. It would just make the Command hold references to other objects for no reason, potentially keeping them from being Garbage collected way earlier.

However, you have to have to provide the CanExecuteChanged event since it is part of the ICommand interface:

public interface ICommand
{
  event EventHandler? CanExecuteChanged;
  bool CanExecute(object? parameter);
  void Execute(object? parameter);
}

What I came up with is to not just not do anything if a user tries to subscribe to the CanExecuteChanged event. I did this by implmenting custom event accessors using the add and remove keywords.

public class ContainerOfNumbers
{
  private ObservableCollection<int> Numbers = new(){1, 2, 3, 4, 5};
  public void ClearPoints() => Numbers.Clear();
}

public class ClearPointsCommand : ICommand
{
  public ClearPointsCommand(ContainerOfNumbers containerOfNumbers)
  {
    _containerOfNumbers = containerOfNumbers;
  }
  private readonly ContainerOfNumbers _containerOfNumbers;
  
  public bool CanExecute(object parameter) => true;
  public void Execute(object parameter) => _containerOfNumbers.ClearPoints();

  public event EventHandler CanExecuteChanged
  {
    add {
    }
    remove {
    }
  }
}

What do you think?

thatguy
  • 21,059
  • 6
  • 30
  • 40
Boris Month
  • 421
  • 6
  • 14
  • Why would you not just use relaycommand? Or delegatecommand. Or any of the other frameworks people already created. – Andy Aug 12 '22 at 14:24

1 Answers1

0

The ICommand interface is part of the WPF framework and defined this way, so there is no way to change how it is handled in general. Of course, you can create custom event accessors and leave them empty, but I consider this premature optimization and there is a lot of debate about it.

As always in this context, you should ask yourself, is there even an issue? Most importantly, did you profile it and gathered evidence? Did you encounter any issues that objects are not collected "way earlier"? How do you define "way earlier" in a garbage collected world? From .NET fundamentals for Garbage Collection:

The garbage collector's optimizing engine determines the best time to perform a collection, based upon the allocations being made. When the garbage collector performs a collection, it checks for objects in the managed heap that are no longer being used by the application and performs the necessary operations to reclaim their memory.

To address your concerns about event subscriptions:

  • In my mind it would be bad to allow subscribing to an event you know will never be fired.

    The framework will call the event accessors nonetheless, whether they are empty or not, so you cannot change that at all. It is in the nature of an event that you cannot tell if it is actually raised or not. That is what it is. From the perspective of the subscriber, am I going to know if an event is ever fired? No at all. As the event source I may have this knowledge, but then again, why do I bother? The event is defined in the interface, which is the contract you have to implement, nothing bad about it.

  • It would just make the Command hold references to other objects for no reason, potentially keeping them from being Garbage collected way earlier.

    This is not really a concern. As I said earlier, you live in a garbage collected world where the garbage collector - which is pretty smart and optimized - decides what is collected when. In regard to events the developers of the WPF developers implemented the Weak Event Pattern exactly for this purpose, because often events are not explicitly unsubscribed. This would definitely result in memory leaks.

    All major mechanisms in WPF use a dedicated weak event manager, e.g.:

    • Binding - Uses PropertyChangedEventManager to subscribe to the PropertyChanged event that is implemented with the INotifyPropertyChanged interface.
    • Collection Bindings - Uses CollectionChangedEventManager to subscribe to the CollectionChanged event that e.g. ObservableCollection<T> implements to notify collection changes.
    • ICommand - Yes, you guessed it, there is a CanExecuteChangedEventManager that manages subscriptions to CanExecuteChanged, so that there are memory leaks, if event handlers are not removed.
    • ...and many more behind the scenes.

Now you see that these kind of weak event subscriptions are very common in WPF. If you think about this micro-optimization of empty event accessors, look at the big picture with all bindings and commands and other mechanisms that rely on events. If this is an issue for you for a subset of commands, then you might have to think of abandoning WPF and .NET in general.

The most common relay and delegate command implementations - be it Prism, MVVMLight, MVVM Toolkit to name a few - boil down to to this and you probably guessed it already:

public event EventHandler CanExecuteChanged;
public DelegateCommand(Action<T> executeMethod) : this(executeMethod, (o) => true)

That being sad, focus on clean, readable code and do not worry about premature optimization.

thatguy
  • 21,059
  • 6
  • 30
  • 40