HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

A more generic `DelegateCommand`

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericdelegatecommandmore

Problem

In an effort to create ever-improving code, I've written a DelegateCommand that takes a generic type parameter to be acted upon. It's pretty trivial, so it should hopefully be easy to review.

In order to do this, I had to create two interfaces: IGenericCommand and IWindowsUniversalCommand.

/// 
/// Requires the implementation of System.Windows.Input.ICommand as well.
/// 
/// 
public interface IGenericCommand : ICommand
{
    /// 
    /// A generic version of System.Windows.Input.ICommand.CanExecute(object).
    /// 
    /// 
    /// 
    bool CanExecute(T parameter);
    /// 
    /// A generic version of System.Windows.Input.ICommand.Execute(object).
    /// 
    /// 
    void Execute(T parameter);
}

/// 
/// Requires the implementation of System.Windows.Input.ICommand as well.
/// 
interface IWindowsUniversalCommand : ICommand
{
    /// 
    /// Should refresh whether or not the ICommand can execute, or call System.Windows.Input.ICommand.CanExecuteChanged.
    /// 
    void RaiseCanExecuteChanged();
}


Then I created an abstract class WindowsUniversalCommand which implements some of the basic logic needed for the command, to save from having to repeat it regularly. (Just in case I create other commands for Windows Universal Apps.)

``
public abstract class WindowsUniversalCommand : IWindowsUniversalCommand
{
public abstract bool CanExecute(object parameter);
public abstract void Execute(object parameter);

#if WINDOWS_UWP
// The .NET version on Windows RT (Universal Apps) does not support the
CommandManager trick below, so we just make a regular event and fire it.
public event EventHandler CanExecuteChanged;
#else
// Programmes that are not Windows Universal Apps support the
CommandManager` trick.
public event EventHandler CanExecuteChanged
{
add { CommandManager.RequerySuggested += value; }
remove { CommandManager.RequerySuggested -= value; }
}
#endif

protected virtual void OnCanExecuteC

Solution

public override bool CanExecute(object parameter) => parameter is T ? CanExecute((T)parameter) : false;


Beware of a lesser known gotcha. This may not work as expected for parameter that's not "really" a T (it's not in Ts inheritance tree), only has an explicit cast operator defined allowing it to be casted to T.

In that case while (T) parameter would return a T instance, CanExecute won't even be called because your is check would prevent this from happening.

Other than that, I'd have some remarks related to naming and code style (rather subjective, so treat them as suggestions or food for thought more than claims your code is incorrect)

public interface IGenericCommand : ICommand


There's no use in putting "generic" in the name. Any C# programmer knows that ` indicates generics by itself. Case in point: in .NET we've got IEnumerable and IEnumerable - not IGenericEnumerable. (So "drop the the" ;) )

/// 

/// 
/// 


In my opinion such empty tags are of no use, all they do is clutter the codebase.

private readonly Func _canExecuteMethod;
private readonly Action _executeMethod;


It's a matter of taste, but here I would ditch the "Method" suffix, as these aren't necessarily "methods". Note that you didn't name the class
MethodCommand. Why not just _canExecute and _onExecute?

public bool CanExecute(T parameter) => _canExecuteMethod == null || _canExecuteMethod.Invoke(parameter);


Since you're using the null safety operator already, this could be simplified to
_canExecuteMethod?.Invoke(parameter) ?? true`.

Code Snippets

public override bool CanExecute(object parameter) => parameter is T ? CanExecute((T)parameter) : false;
public interface IGenericCommand<T> : ICommand
/// <typeparam name="T"></typeparam>

/// <param name="parameter"></param>
/// <returns></returns>
private readonly Func<T, bool> _canExecuteMethod;
private readonly Action<T> _executeMethod;
public bool CanExecute(T parameter) => _canExecuteMethod == null || _canExecuteMethod.Invoke(parameter);

Context

StackExchange Code Review Q#124335, answer score: 4

Revisions (0)

No revisions yet.