Optimizations/refactoring/tweaks to DispatcherQueueHelper #3498
Conversation
The previous version caused the C# compiler to always instantiate a display class for the closure even if the execution had thread access
|
Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request |
| @@ -0,0 +1,251 @@ | |||
| // Licensed to the .NET Foundation under one or more agreements. | |||
michael-hawker
Sep 23, 2020
Member
Did you git mv this file as I'm surprised it didn't mark it as a move...
Did you git mv this file as I'm surprised it didn't mark it as a move...
Sergio0694
Sep 23, 2020
Author
Member
I just moved it as usual through Visual Studio, not sure why it wasn't picked up 🤔
Maybe it's because I also renamed the file in the process?
I just moved it as usual through Visual Studio, not sure why it wasn't picked up
Maybe it's because I also renamed the file in the process?
skendrot
Sep 23, 2020
Contributor
You must move first [commit] and then rename. You can do both in one commit
You must move first [commit] and then rename. You can do both in one commit
Microsoft.Toolkit.Uwp.Connectivity/BluetoothLEHelper/BluetoothLEHelper.cs
Show resolved
Hide resolved
|
This PR has been marked as "needs attention |
|
This PR has been marked as "needs attention |
|
Hello @michael-hawker! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
@azchohfi think we're all good here now? |
Follow up for #3206
PR Type
What kind of change does this PR introduce?
no api changes)What is the new behavior?
This PR includes a number of optimizations, tweaks and refactorings to
DispatcherQueueHelper:nullchecks forfunction, and enabled nullability annotations. Thosenullchecks were unnecessary since there were also other parameters that could also benull(ie. the actualDispatcherQueue) anyway, plus we'd still get an exception anyway when those instances are accessed. Instead, we now have nullability annotations to make the intent clearer.DispatcherQueueExtensions, since these are all extensions and not a helper class.ExtensionsnamespaceEnqueueAsyncto mirror theTryEnqueuename of the actual API in theDispatcherQueueclass. This better follows the naming convention of the class, and it's also clearer as in case users have multiple windows and different dispatching queues, there isn't really a single "UI thread" - the API is literally just enqueueing an operation on a given dispatcher queue.TryEnqueuefails, so if that happens we now return a wrappedInvalidOperationExceptionin theTask, whereas the previous behavior would've just caused that task to never be completed, leaving the caller just waiting forever.PR Checklist
Please check if your PR fulfills the following requirements:
Pull Request has been submitted to the documentation repository instructions. Link:Sample in sample app has been added / updated (for bug fixes / features)Icon has been created (if new sample) following the Thumbnail Style Guide and templatesTests for the changes have been added (for bug fixes / features) (if applicable)