I've been trying to learn async coding recently. I've managed to get it to work and the UI thread doesn't block like it does if I don't make the code async. I realise this is a very contrived example, but is this how async should be done?
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
}
private async void Button_Click(object sender, RoutedEventArgs e)
{
string outputString = (await Authentication(usernameTextBox.Text)) ? "Name exists" : "Name does not exist";
MessageBox.Show(outputString);
}
private Task<bool> Authentication(string username)
{
return Task.Run(() => CheckIfNameExistsInDatabaseOnServer(username));
}
private bool CheckIfNameExistsInDatabaseOnServer(string username)
{
//Pretend to pass details and wait for a response
Thread.Sleep(5000);
return (username == "Will");
}
}
-
1\$\begingroup\$ Your code looks good enough to me. Of course using the Task has much more details and ways that you can dig in. One other resource you might find good about threading in general is this. Also, be careful that you are handling any possible exceptions in the task. This could be a good link, and you will find many other examples on the net and Stack Overflow. \$\endgroup\$A Khudairy– A Khudairy2014年12月24日 12:21:16 +00:00Commented Dec 24, 2014 at 12:21
-
\$\begingroup\$ You need to distinguish between io bound tasks and cpu bound tasks. What you're doing is really an io bound task, as such it is better if you don't spawn another thread but use TaskCompletionSource instead \$\endgroup\$Tien Dinh– Tien Dinh2014年12月27日 16:52:38 +00:00Commented Dec 27, 2014 at 16:52
4 Answers 4
There isn't much to review here, but I would probably have introduced a variable for the await
ed call's result, and then used a ternary to determine the outputString
; the difference would have been that the await
ed call would have stood out more, but there's nothing really wrong with the way you did it.
That said outputString
would probably be better off as outputMessage
, or simply output
- having the type name in an identifier is never a sign of great naming.
I realize it's a tiny little example, but you have everything done in the UI's code-behind here - that's only good for prototype/throw-away code (which this probably is). In a real app you would have coded the logic elsewhere.
you should make use of newlines and the fact that C# is semi-colon terminated right here
private async void Button_Click(object sender, RoutedEventArgs e) { string outputString = (await Authentication(usernameTextBox.Text)) ? "Name exists" : "Name does not exist"; MessageBox.Show(outputString); }
by using newlines and indentations you make your code much more readable (no side-scrolling)
private async void Button_Click(object sender, RoutedEventArgs e)
{
string outputString = (await Authentication(usernameTextBox.Text))
? "Name exists"
: "Name does not exist";
MessageBox.Show(outputString);
}
To follow the Task-based Asynchronous Pattern and usual naming conventions (methods should be verbs, not nouns), your Authentication
method should be named something like AuthenticateAsync
.
If this was a real code, you shouldn't use Task.Run()
on a synchronous method, instead the whole CheckIfNameExistsInDatabaseOnServer
method should be asynchronous, since accessing the DB is IO-bound.
The parentheses in your ternary expression are not necessary (also following Malachi's advice about newlines):
string outputString = await Authentication(usernameTextBox.Text)
? "Name exists"
: "Name does not exist";
Good code for a simple example of async. The real power of async/await comes into play when you have to make service calls, query a database, or you have several things happening at the same time. I think the only reason why you may have questions about whether your example is good enough is because the app is just not very complex.