-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Coding best practices: async vs await improvements
#2156
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
Changes from 37 commits
123c6e8
07c024c
cc37929
d5bc104
9b9ec8c
fae4354
877814c
4c06622
f095153
2031123
551d1b7
ebd5dcb
d82f37a
e708509
4f1ddd1
59bd034
2abdbbf
11c1459
9bd1214
59c3551
63fc393
61d5436
056831f
9a6eab8
1cc02f1
cb21556
8343c38
687a250
9629a94
b6c3d82
a38f733
0c1b38b
6896aaf
a9659ed
480ba72
3655677
b11b8ee
a851098
5ea8c89
efd5a31
16e495f
8061814
5c03c96
943d9bb
93331c7
cc4a920
8b10792
6ff1011
5376507
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,7 @@ public Task<List<Service>> GetAsync() | |
| try | ||
| { | ||
| _logger.LogInformation(() => $"Retrieving new client information for service: {ServiceName}..."); | ||
| _services = _consulServiceDiscoveryProvider.GetAsync().Result; | ||
| _services = _consulServiceDiscoveryProvider.GetAsync().GetAwaiter().GetResult(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are within a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not an issue!.. We will tackle the issue of |
||
| return Task.FromResult(_services); | ||
| } | ||
| finally | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,7 +18,7 @@ private int GetDelay() | |||||||
| { | ||||||||
| var delay = 1000; | ||||||||
|
|
||||||||
| var fileConfig = Task.Run(async () => await _fileConfigurationRepository.Get()).Result; | ||||||||
| var fileConfig = _fileConfigurationRepository.Get().GetAwaiter().GetResult(); | ||||||||
|
||||||||
| int Delay { get; } |
Why not to add async version to the interface?
After that we can make private and public methods as true async →
Task<int> DelayAsync { get; }
| var fileConfig = _fileConfigurationRepository.Get().GetAwaiter().GetResult(); | |
| var fileConfig = await _fileConfigurationRepository.Get(); |
Optional goal is using Task.Delay, if applicable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression has been revised to execute synchronously within the synchronous int GetDelay() method.
The proposed suggestion for refactoring is beyond the scope of this pull request. We will refactor the interface in future.
TODO added in commit 6ff1011 ✔️
Uh oh!
There was an error while loading. Please reload this page.