0

I have a button_click event. which support two methods if async-mode is ON.

So there's for example Update() method which are sync version

And UpdateAsync() method which are async version.

Now Lets see a btnEdit_Click which will call Update() if IsAsynchronous option Boolean is ON.

 internal async void BtnEdit_ItemClick(object sender, ItemClickEventArgs e)
 {  
       if (IsAsynchronousMode)
          await crud.UpdateAsync(crud.DataBuffer);
       else
         crud.Update();
 }

Now is this true what i made ? to make a Method can support both sync / async ? or this anti-pattern. but i assuming the C# events are by default sync. so i put async keyword in the main method.

Also implementation of UpdateAsync() is fine implemented. it call ADO.NET await ExecuteNonQueryAsync()

Sorry am so conflicting and need support both calls in one method. or which is better approach should i follow ?

deveton
  • 291
  • 4
  • 26
  • You could replace the call to `crud.Update();` with `await Task.Run(() => crud.Update());` - but be aware that creates a thread (which imposes some overhead). – Matthew Watson Nov 06 '19 at 15:43
  • 1
    Well the await crud.Update() will be anti-pattern i think i read this before. but about the main click event ? is what i do alright . idk actually. – deveton Nov 06 '19 at 15:45
  • you Compiler should scream at this, an async method should not return void, it always returns a Task – Isparia Nov 06 '19 at 15:47
  • 1
    So, all i got the i should implement one version of Click event for sync. and one version of Click event for async ? is that true or .. ? – deveton Nov 06 '19 at 15:48
  • 1
    -isparia its for Click event only. or should i re-create a control with async version ? – deveton Nov 06 '19 at 15:48
  • Well, i think that old CPU will be great for sync? and Newer CPUs is good for async option. isn't this true – deveton Nov 06 '19 at 16:02
  • Your `crud.Update()` is void, so you cannot use `await Task.FromResult(crud.Update());`. `await Task.Run(crud.Update);` will make it, for your UI, asynchronous (awaitable). Unless you want it synchronous. – Jimi Nov 06 '19 at 16:07
  • @devmuhammadkamal the way to go is in 99% of the usecases the async way, the cpu itself couldnt care less if a method is async or not. Depending on your application it can make a difference, if you want to keep the application responsive after the button click or not, so if you want to go responsive go async, if you dont go sync – Isparia Nov 06 '19 at 16:13
  • Why would you ever have a click handler that might be an async call or might be a in-sync call? This is poor design of your solution, why not just keep the async version only, since you got this far i mean... i doubt there is a business need to have a handler some times go as async and sometimes to run in-sync. Can you explain why you cant just stick with the async version? – MKougiouris Nov 06 '19 at 16:15
  • The only reason I can think of to keep the sync version is because you *want* the click handler to block, e.g. if it is not [reentrant](https://stackoverflow.com/questions/2799023/what-exactly-is-a-reentrant-function), which is actually a pretty good reason. But if that is the case, you don't want the sync version to use `Task.Run()`. – John Wu Nov 06 '19 at 19:01

1 Answers1

2

Marking the method as async just means that the method will have an await somewhere in the implementation. It is not mandatory to return a Task if the method is async, it is only mandatory to have at least an await. You should only return Task if you want to make the method awaitable. I personally have a method like:

public async void Button_Click(object sender, EventArgs e);

So, answering to your question, yes it is true what you've made, the method will only be async when it gets to :

await crud.UpdateAsync(crud.DataBuffer);

otherwise it is sync even though it has async keyword. I just dont get it why you have a sync call when you can have an async

  • Please don't use `async void` if you don't want weird crashes: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#async-void – Claudiu Guiman Nov 06 '19 at 16:15
  • 4
    @ClaudiuGuiman It's generally OK to use `async void` [for event handlers](https://stackoverflow.com/questions/19415646/should-i-avoid-async-void-event-handlers). – Matthew Watson Nov 06 '19 at 16:16
  • @MatthewWatson can you please explain why? Can't you just use a void method (no async) for an event handler that wraps a Task.Run? – Claudiu Guiman Nov 06 '19 at 16:19
  • @ClaudiuGuiman Because that would require you to either 1. block on an async call, and [that can cause problems with deadlocks](https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html), and/or lock the UI thread, or 2. fire and forget, which causes it's own problems with making sure it actually completed successfully. – Gabriel Luci Nov 06 '19 at 16:25
  • Isn't `async void` also fire and forget? – Claudiu Guiman Nov 06 '19 at 16:35
  • @ClaudiuGuiman Yeah, but in this case it's an event handler and it's the form forgetting that your code is running. But it's not the form's job to make sure your code runs to completion - that's your job :) – Gabriel Luci Nov 06 '19 at 16:42
  • 1
    @ClaudiuGuiman events are fire-and-forget too. The code that fires events never waits for them to complete (it doesn't know anything about awaiting), it just calls all subscribed delegates. `async void` was *made specifically* for event handlers. The article you link to is about web applications, not desktop apps and event handlers. – Panagiotis Kanavos Nov 06 '19 at 16:43
  • @ClaudiuGuiman as for why `async void` was created back in 2011-2012, probably because it would cause a ton of problems to do otherwise. For the signature to change, events and event handler delegates themselves would have to change at the language and API level. The firing code would need a way to check whether the subscriber delegates are sync or async and call the async ones ... how? Wait for them to finish? How, unless the firing code *itself* was async? Which means the event handler would have to be async. *Raising* the event would have to be async. Where does this stop, where's the top? – Panagiotis Kanavos Nov 06 '19 at 16:51
  • @ClaudiuGuiman `async void` in event handlers mean the event is the async top – Panagiotis Kanavos Nov 06 '19 at 16:54
  • 1
    Hey luis Filipe. your answer is good for me. i just have for example a long operation sometimes i must need operation to be sync. to make UI unmodified. sometimes make async for example ( updating transactions ) but when i get OR Select. i do sync. to force UI waiting. from what you post i think my btn_Click supports both well ? are am right? – deveton Nov 06 '19 at 19:17
  • 1
    Guys please ? should i change btn_click from void to Task ? or keeping it ?? – deveton Nov 06 '19 at 19:20
  • 1
    The event handler must have void. otherwise Designer.cs error. i think so ? – deveton Nov 06 '19 at 19:22
  • 1
    I suggest you to read my answer again and the comments, and you will have a answer to your question. It is fine to use async void – Luís Filipe Nov 07 '19 at 09:38