Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async command examples #4

Open
brugi82 opened this issue Aug 18, 2016 · 7 comments
Open

Async command examples #4

brugi82 opened this issue Aug 18, 2016 · 7 comments
Assignees

Comments

@brugi82
Copy link

brugi82 commented Aug 18, 2016

Hi Stephen,

This really is not an issue it's just a discussion about usage of AsyncCommand in general.

I really loved your MSDN magazine articles about this subject. They are awesome. You really explained your thought process while creating NotifyTask and AsyncCommand classes. Problem with this version on GitHub is considerable difference than your last explained version. I would really love to see your thought process while designing this class.
Another issue is cancellation. I am not sure what would be the proper way of "connecting" Async and Cancel commands. I did tried to use something like this in ViewModel constructor (same example like you used in your MSDN article):
CancelCountCommand = new CancelCommand();
CountUrlBytesCommand = new AsyncCommand.v2.AsyncCommand(async (param) => ByteCount = await TestService.DownloadAndCountBytesAsync(Url, CancelCountCommand.CancellationToken));

But the problem with this solution is that Cancel button is enabled by default. Canceling works only first time and if user tries to start AsyncCommand again, it fails saying that Task is canceled. How should I use it properly?

Thank you,
Milos

@StephenCleary StephenCleary self-assigned this Aug 20, 2016
@StephenCleary
Copy link
Owner

The NotifyTask type is almost the same as in my article. It really is just a data-bindable Task wrapper, and so it's pretty straightforward.

The AsyncCommand (and the new CancelCommand) are the result of several iterations. This latest iteration splits them out into separate types, each with its own separate responsibilities.

I do plan on writing up docs on using them together, since this will be a common use case. I'm also considering some static methods or possibly simple wrapper types for common construction scenarios.

In the meantime, for a CancelCommand/AsyncCommand pair, use this kind of code:

CancelCountCommand = new CancelCommand();
CancelCountCommand.Cancel();
CountUrlBytesCommand = new AsyncCommand(async _ =>
{
    cancelCommand.Reset();
    try
    {
        await TestService.DownloadAndCountBytesAsync(Url, CancelCountCommand.CancellationToken);
    }
    finally
    {
        cancelCommand.Cancel();
    }
});

@StephenCleary
Copy link
Owner

With the 1.0.0-eta-02 release, the CancelCommand usage is greatly simplified:

CancelCountCommand = new CancelCommand();
CountUrlBytesCommand = new AsyncCommand(CancelCountCommand.WrapDelegate(async (_, token) =>
{
  await TestService.DownloadAndCountBytesAsync(Url, token);
}));

@roubachof
Copy link

So to be clear, while in the previous version we could reuse the CancelCommand, in the 1.0.0-eta-02 we should create a new CancelCommand everytime we use an AsyncCommand ?

@StephenCleary
Copy link
Owner

StephenCleary commented Oct 10, 2016

@roubachof The new CancelCommand can also be reused (note: there are no tests for this behavior yet). The only gotcha is that the CancellationTokenSource is reference-counted, so if you have a situation like this:

CancelCountCommand = new CancelCommand();
CountUrlBytesCommand = new AsyncCommand(CancelCountCommand.WrapDelegate(async (_, token) =>
{
  await TestService.DownloadAndCountBytesAsync(Url, token);
}));

then you can re-use CancelCountCommand only if the CountUrlBytesCommand has completed (that is, the reference count is zero). Triggering the cancellation is insufficient - you'd have to ensure the async command is completely finished.

The problem is, that's difficult to verify in some scenarios (namely, synchronous ones). So I'd say as a general rule, if you're overwriting the CountUrlBytesCommand, then you'd probably want to overwrite CancelCountCommand as well.

It would be possible to make a more-reusable CancelCommand by just dropping the reference count to 0 whenever it's cancelled. Would you want that behavior? I'd have to think about it a bit.

@roubachof
Copy link

roubachof commented Oct 10, 2016

Well what I find peculiar is that with implementation I guess we need to create a new AsyncCommand each time we want to use it in our view.
Consider a scenario when we want to cancel the current command before running a new one.
I mean, historically we use to do something like this:

public class VM 
{     
    public VM()     
    {         
        CountUrlBytesCommand = new AsyncCommand(() => CountUrlBytesCommandAsync);     
    }

    public AsyncCommand CountUrlBytesCommand { get; }

    public async Task CountUrlBytesCommandAsync()    
    {
    }
}

If we have a look to the "old" way (https://msdn.microsoft.com/en-us//magazine/dn630647.aspx), the CancelCommand is directly linked to the current Command.
So from the code behind I could do:

var vm = new VM();
var commandToBeCalled = vm.CountUrlBytesCommand;
var cancelCommand = vm.CountUrlBytesCommand.CancelCommand;
cancelCommand.Execute(null); // Cancels previous before running the new
commandToBeCalled.Execute(...);

So we create the Command once (CancelCommand was embedded in the AsyncCommand).

But with the new approach, I guess we would need to have this:

public class VM 
{     
    public VM()     
    {         
        CancelCommand = new CancelCommand();    
    }

    public CancelCommand CancelCountCommand { get; private set; }

    public AsyncCommand CountUrlBytesCommand 
    {
        get 
        {
            CancelCountCommand = new CancelCommand();
            return new AsyncCommand(CancelCountCommand.WrapDelegate(async (_, token) =>
            {
                await TestService.DownloadAndCountBytesAsync(Url, token);
            }));
        }
    }

    public async Task CountUrlBytesCommandAsync()    
    {
    }
}

And in the code behind if we keep the old workflow:

var vm = new VM();
var commandToBeCalled = vm.CountUrlBytesCommand; // We just created a new instance of AsyncCommand
var cancelCommand = vm.CancelCommand; // Oops old cancel command is gone
cancelCommand.Execute(null); // Cancels the new command
commandToBeCalled.Execute(...); // Born cancelled

So we would have to do:

var vm = new VM();
var cancelCommand = vm.CancelCommand; // the cancel command linked to the previous command
var commandToBeCalled = vm.CountUrlBytesCommand; // We just created a new instance of AsyncCommand
cancelCommand.Execute(null); // Cancels the new command
commandToBeCalled.Execute(...); // Born cancelled

This would be ok but the we are workflow dependent and the accessor creates a new Command.
I'm afraid I'm not very clear :-/

@roubachof
Copy link

So to answer your question: it seems that having a reusable cancel command will fix the workflow + new async command instance issues.

@Spektremouse
Copy link

Any chance some examples of AsyncCommand could be added in conjunction with the IProgress API?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants