AsyncCommand can execute questions

[maxkoshevoi]

1. Why does canExecute parameter for AsyncCommand<T> accept Func<object, bool> instead of Func<T, bool>? It's Func<T, bool> for regular Command<T>.
2. Why does canExecute parameter for AsyncCommand (without T) accept Func<object, bool> instead of Func<bool>? It's Func<bool> for regular Command.
3. Why RaiseCanExecuteChanged() isn't named ChangeCanExecute() to be consistent with similar method from Command?
4. It seems like AsyncCommand<T> should be AsyncCommand<in T> since IAsyncCommand is declared as IAsyncCommand<in T> and we don't return T from anywhere in AsyncCommand<T>

[jfversluis]

@brminnick you are probably the best person to answer this if I'm not mistaken?

[TheCodeTraveler]

No problem!

Since some answers are quite verbose, I'll break them out into separate responses to help with readability 👍

[TheCodeTraveler]

Question 1

Why does canExecute parameter for AsyncCommand<T> accept Func<object, bool> instead of Func<T, bool>? It's Func<T, bool> for regular Command<T>.

Answer

In AsyncCommand<T>, T is the type for the parameter for AsyncCommand.Execute(T parameter).

We purposely did not use T for canExecute<object, bool> to allow for scenarios where CanExecute and and Execute use different parameters. For example, here is a case where we are using an int for the Execute parameter and a bool for the CanExecute parameter:

public ICommand PullToRefreshCommand { get; } = new AsyncCommand((int) count => FetchNewData(count), (bool) isBusy => !isBusy)

This implementation best honors the paradigm of ICommand which doesn't require Execute to use the same parameter type as CanExecute.

I strongly disagree with Xamarin.Forms' original implementation of Command<T> that assigns T to both Execute and CanExecute, because the ICommand Specification does not require Execute and CanExecute to use the same parameter type.

Recommendation

For a type-safe canExecute, I recommend using AsyncCommand<TExecute, TCanExecute> like so:

public ICommand PullToRefreshCommand { get; } = new AsyncCommand<int,bool>(count => FetchNewData(count), isBusy => !isBusy)

[maxkoshevoi]

Oh git it, makes sense. But wouldn't it be better if AsyncCommand<T> was equivalent to AsyncCommand<T, T> and if I needed to have canExecute with different type, I would create command with two type arguments? This way we always have type-safe canExecute and an option for it to be different type if we need to.

[TheCodeTraveler]

It's a fair argument, but it'd be a breaking change to implement it, and the type-safe functionality already exists in AsyncCommand<TExecute, TCanExecute>.

This would be a tough one to advocate for since we'd be introducing a breaking change only to provide a benefit that already exists.

[maxkoshevoi]

Opt-in type-safe is not ideal.. This is new package (according to GitHub only 38 projects use it so far), so there is still time to introduce changes like this. Or this change can be merged on next major release.

If I would need type-safe canExecute (which I assume everyone coding with staticky typed language wants), I'll need to use AsyncCommand<TExecute, TCanExecute> anyway.

if I don't need can execute, I'll use AsyncCommand<TExecute> in both cases.

This change will be breaking only for those who are using not type-safe canExecute (maybe because they knew they could), so it would just point they to more cleaner code.

[TheCodeTraveler]

Question 2

Why does canExecute parameter for AsyncCommand (without T) accept Func<object, bool> instead of Func<bool>? It's Func<bool> for regular Command.

Answer

Similar to the Answer for Question #1, Execute and CanExecute are separate and are not required by the ICommand Specification to use the same parameter type.

I.e. Foregoing an input parameter for Execute does not predicate that you must forego an input parameter for CanExecute.

Here is an example where Execute does not use an input parameter and CanExecute does use an input parameter:

public ICommand PullToRefreshCommand { get; } = new AsyncCommand(() => FetchNewData(count), (bool) isBusy => !isBusy)

Again, I strongly disagree with Xamarin.Forms' implementation of Command as it breaks the ICommand paradigm.

[TheCodeTraveler]

Question 3

Why RaiseCanExecuteChanged() isn't named ChangeCanExecute() to be consistent with similar method from Command?

Answer

Good point! We should include ChangeCanExecute() to provide for an easier transition from Command<T> to AsyncCommand<T>.

The reason I chose the name RaiseCanExecuteChanged is mostly a matter of semantics: RaiseCanExecuteChanged invokes ICommand. CanExecuteChanged; it allows you to Raise (or invoke) the event CanExecute.

tl;dr RaiseCanExecuteChanged is a better description than ChangeCanExecute of what is happening under the hood when the method is called.

Resolution

I'll open a PR to add ChangeCanExecute() 🙌

[TheCodeTraveler]

Update

Pull Request Opened: #736

[maxkoshevoi]

Yes, it sounds better, but backwards compatibility demands sacrifices 😄
In your PR you marked ChangeCanExecute() as Obsolete. Point of my question was that two Command classes should have the same interface so that if some method (that is used to create a command) becomes async (or sync) all programmer needs to do is to change new Command to new AsyncCommand. And right now biggest differences that I noticed are canExecute type (discussed above, so not touching here) and different name of ChangeCanExecute.

[TheCodeTraveler]

Correct, and this PR provides backwards compatibility.

I marked it as [Obsolete] to reduce the maintenance required for two methods that do the same thing, and to reduce the overhead of documenting two methods that do the same thing.

[TheCodeTraveler]

Question 4

It seems like AsyncCommand<T> should be AsyncCommand<in T> since IAsyncCommand is declared as IAsyncCommand<in T> and we don't return T from anywhere in AsyncCommand<T>

Answer

Because <in T> is only allowed for Interfaces and Delegates.

If we try to use <in T> for a class, we get the following compiler error:

The good news is that we are using <in T> for both IAsyncCommand and IAsyncValueCommand!

[maxkoshevoi]

Oh, I missed that part of C#, sorry. Thanks for pointing it out!