[WIP] Dispatcher Helper

[Code-ScottLe]

Addressing issue #471
Things to do:

1. Finalize the methods signature (done)
2. Documentation (after step 1)
3. Examples + Unit Test

[dnfclas]

Hi @Code-ScottLe, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

[dnfclas]

@Code-ScottLe, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[Code-ScottLe]

@deltakosh @lukasf @jlnostr Any feedback regarding the signature of the helpers would be great

[lukasf]

@Code-ScottLe If you ask me, the methods could be combined if "viewToExecuteOn" is last with null defaulting to MainWindow. Then only half the amount of methods would be needed. But more importantly, I would recommend to remove the null propagation. If the methods are called on a null dispatcher or view, then something must have gone wrong. It's better let the dev know by raising a NullReferenceException, rather than silently dropping the call.

[Code-ScottLe]

@lukasf Personally i prefer the overloaded method for handling the scenario of default view to be main view. It makes the code from the user stand point cleaner and easier to use than having to explain to them that they have to pass null in for the viewToExecuteOn. Definitely open up to change if that is something that the community desire to have. Just my 2 cents

[lukasf]

@Code-ScottLe Agreed, maybe this is the better way to do it explicitly here. As i said, I have more concerns about the null propagation dropping developer errors silently.

[Code-ScottLe]

@lukasf That is one of the thing that i noticed after the PR yesterday, that the call would fail silently in the case of null view. I think we should perform a check and throw NullAgrumentException there or throwing NullReferenceException like you said too.

[deltakosh]

Agree on the exception. We cannot swallow error.
Can you also provide an sample and a doc?

[Code-ScottLe]

@deltakosh Look like we don't have any problem with the signature of the helpers. I will make example and document for it very soon. The null check will be done along with it

[deltakosh]

I confirm: API looks good to me

[deltakosh]

Just one question:
What is the difference between:

• await CoreDispatcher.RunAsync()
• await CoreDispatcher.AwaitableRunAsync()

[Code-ScottLe]

RunAsync() is the default, built-in method of CoreDispatcher. It merely schedule the given delegate to run and return immediately, so the await on RunAsync is not truly awaitable.

AwaitableRunAsync() was created to solve this problem, an actual awaitable Task or Task<T> for return type T (which RunAsync() lacks)

[deltakosh]

Ok so perhaps the name should be clearer (at least to me)

[Code-ScottLe]

Any others bright ideas for naming? I'm not too comfortable with that name TBH, but that is the best one that i can come up with.

[deltakosh]

Should I bring the fact that I'm french? :)
Let me ping more capable people ;)
@dotMorten @skendrot @ScottIsAFool @hermitdave

[hermitdave]

How about ScheduleAsync and ExecuteAsync ?

[hermitdave]

sorry last in terms of capability but first in terms of time :)

[Code-ScottLe]

Good point on context switching by @dotMorten , i will fix it on the next commit

[Code-ScottLe]

@dotMorten is this good to go yet?

@deltakosh I am starting to do the documentation :) , I don't know if i can unit test this thing though.

[Code-ScottLe]

Alright, I have decided to add in support for synchronous code for the API as well (through Func<T> and Action), to counter the scenario of user wanting to do a quick short lambda on UI thread before going straight back to what they are doing. I think it is a good addition to solve cases like this (There were also cases that I have seen in the past , of synchronous APIs that are required to be run in UI thread, usually graphic-related APIs):

    await DispatcherHelper.ExecuteOnUIThreadAsync(() =>{
    NormalTextBlock.Text = "Updated from a random thread!";
});

Before having support for synchronous code, they have to tag the lambda async and return null, which is a bit clunky and hard to document imo. Thoughts? @deltakosh @dotMorten @lukasf

[lukasf]

@Code-ScottLe Excellent, I did not realize that this was missing, it sure makes sense.

[deltakosh]

So something like DispatcherHelper.ExecuteOnUIThreadAndReturnImmediatly? (with a good name)

based on how WinRT is build I think we should not do it because we should do it for every async method then.

beside that I like the helper as it is right now

[Code-ScottLe]

It does not return immediately though, as it will still propagate back a Task or Task<T> that is awaitable in true manner for the non-UI / caller thread (as the result will have to make it back to the inner TaskCompletionSource.SetResult() before AwaitableRunAsync finish on the caller thread.

I ran into the same trouble with the Microsoft Band SDK before, as the extension method they built to handle the bitmap was synchronous and required to be run under UI Thread. Of course i can still make an async lambda and return null, but it was just too clunky and Visual Studio will yell at you for having an async function without an "await" keyword. Definitely open up for changes though

[deltakosh]

Because we are close to code freeze, I would suggest to leave it like this and think about adding sync methods for 1.3.

Does it make sense?

[Code-ScottLe]

I have already added it in, do you want me to remove it? :D

It was done along side with the documentation and sample page (as this problem was coming from making the sample page)

[deltakosh]

I must be missing something but I do not see sync methods? Should I? (I guess so based on your comment :D)

[Code-ScottLe]

Ah, we are not talking about adding a synchronous version of the AwaitableRunAsync as a synchronous function, it was the parameter for AwaitableRunAsync that needs synchronous support (in other word, the user-specified block of code to be run in UI thread, not the API itself).

Before, AwaitableRunAsync supports only functions that returns either Task or Task<T> , and this raise the problem if the user wants to do a quick UI update via a synchronous lambda like () => TextBox1.Text = "hahaha" , which has the return type of void . Without the support for synchronous code, the call would look like this:

Dispatcher.AwaitableRunAsync( async () => { TextBox1.Text = "hahaha"; return null; } );

So by adding an overload of AwaitableRunAsync, we will be able to eliminate the need of the async keyword and the return null in the example above.

[deltakosh]

Ok! gotcha
So fine for me:)

[ScottIsAFool]

Sorry @Code-ScottLe, I think a merge I've just done has now given this conflicts.

[Code-ScottLe]

@ScottIsAFool Look like we just merge in a change that has new sample page. No worries, I will fix it.

[Code-ScottLe]

@ScottIsAFool Try again now.

[ScottIsAFool]

Quick question, why isn't this an extension method?

[Code-ScottLe]

It takes away the knowledge that user have to know about CoreDispatcher. I wanted to provide a quick and simple way to do execute something under UI thread without having to fiddle with where the Dispatcher is.

[ScottIsAFool]

I didn't mean an extension method for coreDispatcher, I meant an extension method for CoreApplicationView :)

[Code-ScottLe]

I don't think we gain much from doing:
CoreApplication.MainView.Dispatcher.AwaitableRunAsync to
CoreApplication.MainView.ExecuteOnUIThreadAsync

Even though we are ... 5 letter shorter :D

[ScottIsAFool]

No, that's not what that method is :P you're right with what im suggesting, CoreApplication.MainView.ExecuteOnUIThreadAsync, but I'm saying that it's instead of
DispatcherHelper.ExecuteOnUIThreadAsync(CoreApplication.Mainview, myFunc)

[lukasf]

I also think that this should be an extension method. You can still run it through DispatcherHelper if you like. But why not make it callable directly on the CoreApplicationView as well? myApplicationView.ExecuteOnUIThreadAsync(func) sounds better to me, compared to DispatcherHelper.ExecuteOnUIThreadAsync(myApplicationView, func).

[Code-ScottLe]

Hmm, that is also a valid point. We can support both, there is no harm as anybody could use any options that they would like and the outcome would be the same. I'll add it in for 1.3 then (since this has already been merged?) cc @deltakosh

[lukasf]

Sure, this can wait for 1.3, no need to rush it (won't be a breaking change)...

[lukasf]

Looks like we still have a few days to add to 1.2.

[Code-ScottLe]

I should be able to do it tonight.

[dotMorten]

I don't understand why you're saying you have to return null. That makes sense if you expect Task<object> as a parameter, but that wouldn't be the case if you merely take Task

[lukasf]

Still you would need to either create an "async" lambda (although you do not execute anything async), or you would need to return a Task.FromResult(null), to satisfy the Func<Task> signature.Both are unneccessary with the new Action and Func<T> overloads, so this is appreciated.

[deltakosh]

We can still push PR until this week end