Replace APM-based code with equivalent async/await-based code #19

Merged
merged 6 commits into from Aug 7, 2013

Conversation

Projects
None yet
2 participants
@dlhartveld
Contributor

dlhartveld commented Aug 6, 2013

As part of a research project at the University of Illinois at Urbana-Champaign, we're developing a refactoring tool that replaces instances of the APM pattern with corresponding TAP method calls and the async/await keywords. I've applied it to this project and found four calls to APM methods.

This pull request replaces those existing calls to APM Begin/End* methods with functionally equivalent TAP constructs and the async/await keywords.

See the following two commits specifically for the actual code-level changes:

The rest of the pull request is mostly related to NuGet package restore. I've enabled it, otherwise VS 2012 wouldn't allow me to add another dependency that is needed: Microsoft.Bcl.Async is a package that adds support for async/await-based programming to older frameworks (Such as WP). As the packages/ directory is currently under version control, I've tried to keep it as clean as possible (i.e. I've removed all non-'sl4-windowsphone71' directories).

Are you interested in merging this pull request? If not, please let me know why, and I'll try and improve the pull request with your comments in mind.

Thanks for your time,
David Hartveld

@ghost ghost assigned mikkoviitala Aug 7, 2013

mikkoviitala pushed a commit that referenced this pull request Aug 7, 2013

Mikko Viitala
Merge pull request #19 from dlhartveld/replace-apm-with-aa
Replace APM-based code with equivalent async/await-based code

@mikkoviitala mikkoviitala merged commit fc28a61 into mikkoviitala:master Aug 7, 2013

@mikkoviitala

This comment has been minimized.

Show comment
Hide comment
@mikkoviitala

mikkoviitala Aug 7, 2013

Owner

Hey,

Thanks mate, I merged your commits.

One thing it missed though, is in MainViewModel class constructor.

There is no await applied to following bit of code, even though RetrieveServerMessage is now async task:

(new DownloadService(ViewModelLocator.CookieJar)).RetrieveServerMessage(Common.ServerMessageUrl)

Thanks again and good luck with your project!

BR,

Mikko Viitala

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎1‎:‎29
Vast.ott: mikkoviitala/battlelogmobile

As part of a research project at the University of Illinois at Urbana-Champaign, we're developing a refactoring tool that replaces instances of the APM pattern with corresponding TAP method calls and the async/await keywords. I've applied it to this project and found four calls to APM methods.

This pull request replaces those existing calls to APM Begin/End* methods with functionally equivalent TAP constructs and the async/await keywords.

See the following two commits specifically for the actual code-level changes:
e55150f
9f1fe35

The rest of the pull request is mostly related to NuGet package restore. I've enabled it, otherwise VS 2012 wouldn't allow me to add another dependency that is needed: Microsoft.Bcl.Async is a package that adds support for async/await-based programming to older frameworks (Such as WP). As the packages/ directory is currently under version control, I've tried to keep it as clean as possible (i.e. I've removed all non-'sl4-windowsphone71' directories).

Are you interested in merging this pull request? If not, please let me know why, and I'll try and improve the pull request with your comments in mind.

Thanks for your time,
David Hartveld

You can merge this Pull Request by running
git pull https://github.com/dlhartveld/battlelogmobile replace-apm-with-aa

Or view, comment on, or merge it at:

#19

Commit Summary
Enabled NuGet package restore.
Restored packages. Added Microsoft.Bcl.Async as dependency.
Added NuGet dependency on Microsoft.Bcl.Async in BattlelogMobile.Client.
Replace APM method calls with async/await-based versions.
Replaced two APM instances with async/await-based code.
Removed unnecessary NuGet files.

File Changes
A .nuget/NuGet.Config (6)
A .nuget/NuGet.exe (0)
A .nuget/NuGet.targets (133)
M BattlelogMobile.Client/BattlelogMobile.Client.csproj (23)
M BattlelogMobile.Client/ViewModel/MainViewModel.cs (80)
M BattlelogMobile.Client/packages.config (3)
M BattlelogMobile.Core/BattlelogMobile.Core.csproj (19)
M BattlelogMobile.Core/BattlelogMobile.Core.csproj.user (3)
M BattlelogMobile.Core/Service/DownloadService.cs (61)
M BattlelogMobile.Core/packages.config (3)
M BattlelogMobile.sln (11)
M BattlelogMobile.v11.suo (0)
A packages/MetroGridHelper.1.11/MetroGridHelper.1.11.nupkg (0)
A packages/Microsoft.Bcl.1.0.19/License.rtf (505)
A packages/Microsoft.Bcl.1.0.19/Microsoft.Bcl.1.0.19.nupkg (0)
A packages/Microsoft.Bcl.1.0.19/Microsoft.Bcl.1.0.19.nuspec (34)
A packages/Microsoft.Bcl.1.0.19/ReleaseNotes.txt (24)
A packages/Microsoft.Bcl.1.0.19/content/net45/. (0)
A packages/Microsoft.Bcl.1.0.19/content/portable-net45+win8+wp8/. (0)
A packages/Microsoft.Bcl.1.0.19/content/sl4/. (0)
A packages/Microsoft.Bcl.1.0.19/content/sl5/. (0)
A packages/Microsoft.Bcl.1.0.19/content/win8/. (0)
A packages/Microsoft.Bcl.1.0.19/content/wp8/. (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Runtime.dll (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Runtime.xml (860)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Threading.Tasks.dll (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Threading.Tasks.xml (8969)
A packages/Microsoft.Bcl.Async.1.0.16/License.rtf (522)
A packages/Microsoft.Bcl.Async.1.0.16/Microsoft.Bcl.Async.1.0.16.nupkg (0)
A packages/Microsoft.Bcl.Async.1.0.16/Microsoft.Bcl.Async.1.0.16.nuspec (28)
A packages/Microsoft.Bcl.Async.1.0.16/ReleaseNotes.txt (18)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.Phone.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.Phone.xml (141)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.xml (275)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.xml (630)
A packages/Microsoft.Bcl.Build.1.0.8/License.rtf (505)
A packages/Microsoft.Bcl.Build.1.0.8/Microsoft.Bcl.Build.1.0.8.nupkg (0)
A packages/Microsoft.Bcl.Build.1.0.8/Microsoft.Bcl.Build.1.0.8.nuspec (18)
A packages/Microsoft.Bcl.Build.1.0.8/content/sl4-windowsphone71/. (0)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Install.ps1 (18)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Microsoft.Bcl.Build.Tasks.dll (0)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Microsoft.Bcl.Build.targets (227)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Uninstall.ps1 (15)
A packages/MvvmLight.3.1.1/MvvmLight.3.1.1.nupkg (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.Extras.WP71.pdb (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.Extras.WP71.xml (142)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.WP71.pdb (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.WP71.xml (1104)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/System.Windows.Interactivity.xml (1025)
A packages/Newtonsoft.Json.4.0.7/Newtonsoft.Json.4.0.7.nupkg (0)
A packages/Newtonsoft.Json.4.0.7/lib/sl4-windowsphone71/Newtonsoft.Json.pdb (0)
A packages/Newtonsoft.Json.4.0.7/lib/sl4-windowsphone71/Newtonsoft.Json.xml (6394)
A packages/SharpGIS.GZipWebClient.1.2/SharpGIS.GZipWebClient.1.2.nupkg (0)
D packages/SharpSerializer.2.18/lib/net20-cf/Polenter.SharpSerializer.Compact.dll (0)
D packages/SharpSerializer.2.18/lib/net20-cf/Polenter.SharpSerializer.Compact.xml (2665)
D packages/SharpSerializer.2.18/lib/net20/Polenter.SharpSerializer.XML (2640)
D packages/SharpSerializer.2.18/lib/net20/Polenter.SharpSerializer.dll (0)
D packages/SharpSerializer.2.18/lib/sl3-wp/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl3-wp/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl3/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl3/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl4-windowsphone/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl4-windowsphone/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl4/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl4/Polenter.SharpSerializer.Silverlight.xml (2622)
A packages/SilverlightToolkitWP.4.2011.12.14/SilverlightToolkitWP.4.2011.12.14.nupkg (0)
A packages/WP7-CI.1.1/WP7-CI.1.1.nupkg (0)
A packages/WP7-CI.1.1/content/README-WP7-CI.txt (7)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.Silverlight.Testing.dll.CodeAnalysisLog.xml (285)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.Silverlight.Testing.xml (14927)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.VisualStudio.QualityTools.UnitTesting.Silverlight.xml (3429)
A packages/WP7-CI.1.1/tools/Install.ps1 (14)
A packages/WP7-CI.1.1/tools/Microsoft.Silverlight.Testing.Tools.dll (0)
A packages/WP7-CI.1.1/tools/Microsoft.Silverlight.Toolkit.Build.Tasks.dll (0)
A packages/WP7-CI.1.1/tools/Uninstall.ps1 (16)
A packages/WP7-CI.1.1/tools/WP7CI.targets (29)
A packages/YLAD.1.4/YLAD.1.4.nupkg (0)
A packages/YLAD.1.4/lib/sl40-wp71/YourLastAboutDialog.xml (518)
A sl4-windowsphone71/ensureRedirect.xml b/packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/ensureRedirect.xml (0)

Patch Links:
https://github.com/mikkoviitala/battlelogmobile/pull/19.patch
https://github.com/mikkoviitala/battlelogmobile/pull/19.diff

Owner

mikkoviitala commented Aug 7, 2013

Hey,

Thanks mate, I merged your commits.

One thing it missed though, is in MainViewModel class constructor.

There is no await applied to following bit of code, even though RetrieveServerMessage is now async task:

(new DownloadService(ViewModelLocator.CookieJar)).RetrieveServerMessage(Common.ServerMessageUrl)

Thanks again and good luck with your project!

BR,

Mikko Viitala

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎1‎:‎29
Vast.ott: mikkoviitala/battlelogmobile

As part of a research project at the University of Illinois at Urbana-Champaign, we're developing a refactoring tool that replaces instances of the APM pattern with corresponding TAP method calls and the async/await keywords. I've applied it to this project and found four calls to APM methods.

This pull request replaces those existing calls to APM Begin/End* methods with functionally equivalent TAP constructs and the async/await keywords.

See the following two commits specifically for the actual code-level changes:
e55150f
9f1fe35

The rest of the pull request is mostly related to NuGet package restore. I've enabled it, otherwise VS 2012 wouldn't allow me to add another dependency that is needed: Microsoft.Bcl.Async is a package that adds support for async/await-based programming to older frameworks (Such as WP). As the packages/ directory is currently under version control, I've tried to keep it as clean as possible (i.e. I've removed all non-'sl4-windowsphone71' directories).

Are you interested in merging this pull request? If not, please let me know why, and I'll try and improve the pull request with your comments in mind.

Thanks for your time,
David Hartveld

You can merge this Pull Request by running
git pull https://github.com/dlhartveld/battlelogmobile replace-apm-with-aa

Or view, comment on, or merge it at:

#19

Commit Summary
Enabled NuGet package restore.
Restored packages. Added Microsoft.Bcl.Async as dependency.
Added NuGet dependency on Microsoft.Bcl.Async in BattlelogMobile.Client.
Replace APM method calls with async/await-based versions.
Replaced two APM instances with async/await-based code.
Removed unnecessary NuGet files.

File Changes
A .nuget/NuGet.Config (6)
A .nuget/NuGet.exe (0)
A .nuget/NuGet.targets (133)
M BattlelogMobile.Client/BattlelogMobile.Client.csproj (23)
M BattlelogMobile.Client/ViewModel/MainViewModel.cs (80)
M BattlelogMobile.Client/packages.config (3)
M BattlelogMobile.Core/BattlelogMobile.Core.csproj (19)
M BattlelogMobile.Core/BattlelogMobile.Core.csproj.user (3)
M BattlelogMobile.Core/Service/DownloadService.cs (61)
M BattlelogMobile.Core/packages.config (3)
M BattlelogMobile.sln (11)
M BattlelogMobile.v11.suo (0)
A packages/MetroGridHelper.1.11/MetroGridHelper.1.11.nupkg (0)
A packages/Microsoft.Bcl.1.0.19/License.rtf (505)
A packages/Microsoft.Bcl.1.0.19/Microsoft.Bcl.1.0.19.nupkg (0)
A packages/Microsoft.Bcl.1.0.19/Microsoft.Bcl.1.0.19.nuspec (34)
A packages/Microsoft.Bcl.1.0.19/ReleaseNotes.txt (24)
A packages/Microsoft.Bcl.1.0.19/content/net45/. (0)
A packages/Microsoft.Bcl.1.0.19/content/portable-net45+win8+wp8/. (0)
A packages/Microsoft.Bcl.1.0.19/content/sl4/. (0)
A packages/Microsoft.Bcl.1.0.19/content/sl5/. (0)
A packages/Microsoft.Bcl.1.0.19/content/win8/. (0)
A packages/Microsoft.Bcl.1.0.19/content/wp8/. (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Runtime.dll (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Runtime.xml (860)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Threading.Tasks.dll (0)
A packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/System.Threading.Tasks.xml (8969)
A packages/Microsoft.Bcl.Async.1.0.16/License.rtf (522)
A packages/Microsoft.Bcl.Async.1.0.16/Microsoft.Bcl.Async.1.0.16.nupkg (0)
A packages/Microsoft.Bcl.Async.1.0.16/Microsoft.Bcl.Async.1.0.16.nuspec (28)
A packages/Microsoft.Bcl.Async.1.0.16/ReleaseNotes.txt (18)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.Phone.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.Phone.xml (141)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.Extensions.xml (275)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.dll (0)
A packages/Microsoft.Bcl.Async.1.0.16/lib/sl4-windowsphone71/Microsoft.Threading.Tasks.xml (630)
A packages/Microsoft.Bcl.Build.1.0.8/License.rtf (505)
A packages/Microsoft.Bcl.Build.1.0.8/Microsoft.Bcl.Build.1.0.8.nupkg (0)
A packages/Microsoft.Bcl.Build.1.0.8/Microsoft.Bcl.Build.1.0.8.nuspec (18)
A packages/Microsoft.Bcl.Build.1.0.8/content/sl4-windowsphone71/. (0)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Install.ps1 (18)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Microsoft.Bcl.Build.Tasks.dll (0)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Microsoft.Bcl.Build.targets (227)
A packages/Microsoft.Bcl.Build.1.0.8/tools/Uninstall.ps1 (15)
A packages/MvvmLight.3.1.1/MvvmLight.3.1.1.nupkg (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.Extras.WP71.pdb (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.Extras.WP71.xml (142)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.WP71.pdb (0)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/GalaSoft.MvvmLight.WP71.xml (1104)
A packages/MvvmLight.3.1.1/lib/sl4-windowsphone71/System.Windows.Interactivity.xml (1025)
A packages/Newtonsoft.Json.4.0.7/Newtonsoft.Json.4.0.7.nupkg (0)
A packages/Newtonsoft.Json.4.0.7/lib/sl4-windowsphone71/Newtonsoft.Json.pdb (0)
A packages/Newtonsoft.Json.4.0.7/lib/sl4-windowsphone71/Newtonsoft.Json.xml (6394)
A packages/SharpGIS.GZipWebClient.1.2/SharpGIS.GZipWebClient.1.2.nupkg (0)
D packages/SharpSerializer.2.18/lib/net20-cf/Polenter.SharpSerializer.Compact.dll (0)
D packages/SharpSerializer.2.18/lib/net20-cf/Polenter.SharpSerializer.Compact.xml (2665)
D packages/SharpSerializer.2.18/lib/net20/Polenter.SharpSerializer.XML (2640)
D packages/SharpSerializer.2.18/lib/net20/Polenter.SharpSerializer.dll (0)
D packages/SharpSerializer.2.18/lib/sl3-wp/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl3-wp/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl3/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl3/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl4-windowsphone/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl4-windowsphone/Polenter.SharpSerializer.Silverlight.xml (2622)
D packages/SharpSerializer.2.18/lib/sl4/Polenter.SharpSerializer.Silverlight.dll (0)
D packages/SharpSerializer.2.18/lib/sl4/Polenter.SharpSerializer.Silverlight.xml (2622)
A packages/SilverlightToolkitWP.4.2011.12.14/SilverlightToolkitWP.4.2011.12.14.nupkg (0)
A packages/WP7-CI.1.1/WP7-CI.1.1.nupkg (0)
A packages/WP7-CI.1.1/content/README-WP7-CI.txt (7)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.Silverlight.Testing.dll.CodeAnalysisLog.xml (285)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.Silverlight.Testing.xml (14927)
A packages/WP7-CI.1.1/lib/sl3-wp/Microsoft.VisualStudio.QualityTools.UnitTesting.Silverlight.xml (3429)
A packages/WP7-CI.1.1/tools/Install.ps1 (14)
A packages/WP7-CI.1.1/tools/Microsoft.Silverlight.Testing.Tools.dll (0)
A packages/WP7-CI.1.1/tools/Microsoft.Silverlight.Toolkit.Build.Tasks.dll (0)
A packages/WP7-CI.1.1/tools/Uninstall.ps1 (16)
A packages/WP7-CI.1.1/tools/WP7CI.targets (29)
A packages/YLAD.1.4/YLAD.1.4.nupkg (0)
A packages/YLAD.1.4/lib/sl40-wp71/YourLastAboutDialog.xml (518)
A sl4-windowsphone71/ensureRedirect.xml b/packages/Microsoft.Bcl.1.0.19/lib/sl4-windowsphone71/ensureRedirect.xml (0)

Patch Links:
https://github.com/mikkoviitala/battlelogmobile/pull/19.patch
https://github.com/mikkoviitala/battlelogmobile/pull/19.diff

@dlhartveld

This comment has been minimized.

Show comment
Hide comment
@dlhartveld

dlhartveld Aug 7, 2013

Contributor

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!

Contributor

dlhartveld commented Aug 7, 2013

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!

@mikkoviitala

This comment has been minimized.

Show comment
Hide comment
@mikkoviitala

mikkoviitala Aug 21, 2013

Owner

Hey,

See what I received today :D !

Maybe you could talk to each other since you seem to be interested in same thing.

BR,

Mikko Viitala

Hi,

I am Semih Okur, a PhD student in the CS department at the University of Illinois. I'm currently doing research on asynchronous programming in phone applications. I found many misuses of async/await keywords in Windows Phone applications. One typical misuse is that developers use synchronous method calls for expensive or IO bound calls in async methods. However, these synchronous method calls have corresponding asynchronous versions. An example from your application, battlelogmobile:

public async Task RetrieveServerMessage(string url)

    {

            ...

            var reader = new StreamReader(responseStream);

            string message = reader.ReadToEnd();

            ...

}

Instead of reader.ReadToEnd(), await reader.ReadToEndAsync() might be used. There are similar opportunities in your app.

I can create a pull request for the patch if you are interested. It is very important for me that you accept the pull request if you think that it is useful. One of the ways to demonstrate my research is to show that patches are accepted. Please let me know.

Thanks,

Semih Okur

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎20‎:‎48
Vast.ott: mikkoviitala/battlelogmobile
Kopio: Mikko Viitala

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!


Reply to this email directly or view it on GitHub.

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎20‎:‎48
Vast.ott: mikkoviitala/battlelogmobile
Kopio: Mikko Viitala

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!


Reply to this email directly or view it on GitHub.

Owner

mikkoviitala commented Aug 21, 2013

Hey,

See what I received today :D !

Maybe you could talk to each other since you seem to be interested in same thing.

BR,

Mikko Viitala

Hi,

I am Semih Okur, a PhD student in the CS department at the University of Illinois. I'm currently doing research on asynchronous programming in phone applications. I found many misuses of async/await keywords in Windows Phone applications. One typical misuse is that developers use synchronous method calls for expensive or IO bound calls in async methods. However, these synchronous method calls have corresponding asynchronous versions. An example from your application, battlelogmobile:

public async Task RetrieveServerMessage(string url)

    {

            ...

            var reader = new StreamReader(responseStream);

            string message = reader.ReadToEnd();

            ...

}

Instead of reader.ReadToEnd(), await reader.ReadToEndAsync() might be used. There are similar opportunities in your app.

I can create a pull request for the patch if you are interested. It is very important for me that you accept the pull request if you think that it is useful. One of the ways to demonstrate my research is to show that patches are accepted. Please let me know.

Thanks,

Semih Okur

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎20‎:‎48
Vast.ott: mikkoviitala/battlelogmobile
Kopio: Mikko Viitala

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!


Reply to this email directly or view it on GitHub.

Lähettäjä: David Hartveld
Lähetetty: ‎keskiviikko‎, ‎7‎. ‎elokuu‎ta ‎2013 ‎20‎:‎48
Vast.ott: mikkoviitala/battlelogmobile
Kopio: Mikko Viitala

Thanks for the feedback! We've chosen to introduce the Task return type for methods that originally return void, but it is not necessary to await it to retain behavior as it was in the original code: RetrieveServerMessage is a 'fire-and-forget' method (i.e., once started, it runs and the caller does not synchronize on it). Introducing Task merely allows for future 'awaiting' of the operation.

As for this particular case, the problem is that constructors cannot be async - which makes sense, because you have to be very careful with starting background work in it: a reference to the object under construction, while not yet constructed completely, could escape to another thread. In this case I think you're good, but it's something to keep in mind. The general solution would be to extract the call(s) that start background work into another method, and call that after the object is created.

Again, thanks for your feedback!


Reply to this email directly or view it on GitHub.

@dlhartveld

This comment has been minimized.

Show comment
Hide comment
@dlhartveld

dlhartveld Aug 21, 2013

Contributor

Yes, we should have made this more explicit - Semih is actually my project partner, also from UIUC. We're working on two different things: APM-to-async/await refactorings, and on analyzing misuse, or ineffective use, of async/await.

In this case, the refactoring introduced an ineffective use of async/await. In a second analysis run, Semih found the use of the synchronous method, that can be replaced by the asynchronous version. This will most likely increase the responsiveness of the UI (i.e., it won't look to be freezing).

Thanks for giving me a ping! We hope you find our suggestions useful, because that means our research makes an impact on practical software engineering as well!

Regards,
David

Contributor

dlhartveld commented Aug 21, 2013

Yes, we should have made this more explicit - Semih is actually my project partner, also from UIUC. We're working on two different things: APM-to-async/await refactorings, and on analyzing misuse, or ineffective use, of async/await.

In this case, the refactoring introduced an ineffective use of async/await. In a second analysis run, Semih found the use of the synchronous method, that can be replaced by the asynchronous version. This will most likely increase the responsiveness of the UI (i.e., it won't look to be freezing).

Thanks for giving me a ping! We hope you find our suggestions useful, because that means our research makes an impact on practical software engineering as well!

Regards,
David

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment