Can I refactor code to fix "Asynchronous method should not return void"?

Question

I am doing a project in c# using Xamarin. The compiler warns me that "Asynchronous method 'HandleWidget_ClickButton' should not return void". See example here:

//Code is simplified

public class Widget {
   public event Action<int> ClickButton;

   private void FireClickButton (int id)
   {
       if (ClickButton != null) {
           ClickButton (id);
       }
   }

   //somewhere else i call FireClickButton(1);
}

public class MyFragment {
    private _widget Widget;

    public override View OnCreateView (LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState)
    {
        //...
        _widget = view.FindViewById<Widget> (Resource.Id.widget);
        //...
    }


    public override void OnResume ()
    {
        base.OnResume ();
        _widget.ClickButton += HandleWidget_ClickButton;
    }

    async void HandleWidget_ClickButton (int id)
    {
        await SaveSomethingInStorage (id);
    }
}

Can I somehow return something to an event/action/delegate? I do not know if this is possible, or how to write this, syntax-wise, and I've spent quite some time searching for a solution. Elsewhere I read that when handling events, it's ok to use async void (instead of something like async Task), but I like to avoid warnings and do not like to hide them using #Pragma instructions.

EDIT (With answer) from @hvd:

You created an event handler, but you don't follow the .NET conventions for event handlers. You should. The warning is automatically suppressed for what can be detected as event handlers.

The .NET conventions for event handlers require a sender, of type object, and the event arguments, of type EventArgs or a class derived from EventArgs. If you use this signature, you should no longer get a warning.

I rewrote my code like this:

public class MyEventArgs : EventArgs
{
    public MyEventArgs (int id)
    {
        ID = id;
    }

    public int ID;
}

public class Widget {
    public event EventHandler<MyEventArgs> ClickTest;

    void FireClickButton (int id)
    {
        if (ClickTest != null) {
            ClickTest (this, new MyEventArgs (id));
        }
    }
}

//In observer class
_widget.ClickTest += HandleWidget_ClickTest;

async void HandleWidget_ClickTest (object sender, MyEventArgs e)
{
    await DoSomethingAsync (e.ID);
}

Note that you must derive from EventArgs. Doing it like this does not suppress the warning:

public event EventHandler<int> AnotherClickTest;

if (AnotherClickTest != null) {
   AnotherClickTest (this, 1);
}

You could return `Task` instead. But event handlers are the exception to that rule - https://msdn.microsoft.com/en-us/magazine/jj991977.aspx — smoksnes, Nov 23 '16 at 11:22
Also, take a look at this question - http://stackoverflow.com/questions/27282617/is-it-safe-to-use-async-await-in-asp-net-event-handlers — smoksnes, Nov 23 '16 at 11:45

score 1 · Answer 1 · answered Nov 23 '16 at 11:25

1

Async methods always should return a Task. A Task represents an action that has not finished yet. Afterwards you can do a Wait() on a task, to get the finished result... since your method is Void, you won't do any wait. The signature should look like this:

async Task HandleWidget_ClickButton (int id)

If you wanted to return something, it would be like this (for example, int):

async Task<int> HandleWidget_ClickButton (int id)

answered Nov 23 '16 at 11:25

Vlad

536
1
5
18

I understand. However, when I refactor my code using your suggestion, I'm faced with the problem that I also need to refactor my event, and I do not know how to do that. For example, when using "async Task HandleWidget_ClickButton (int id)", the code where I bind (_widget.ClickButton += HandleWidget_ClickButton) gives me the error "Task HandleWidget_ClickButton has the wrong return type". – Gerhard Schreurs Nov 23 '16 at 12:06

score 1 · Accepted Answer · answered Nov 23 '16 at 11:28

1

You created an event handler, but you don't follow the .NET conventions for event handlers. You should. The warning is automatically suppressed for what can be detected as event handlers.

The .NET conventions for event handlers require a sender, of type object, and the event arguments, of type EventArgs or a class derived from EventArgs. If you use this signature, you should no longer get a warning.

answered Nov 23 '16 at 11:28

Actually, in VS2015 I don't get any warnings about this at all. – smoksnes Nov 23 '16 at 11:45
@smoksnes I do. There are multiple extensions to VS that provide additional warnings, it may be coming from one of those. I'll check where it's coming from in my case. – Nov 23 '16 at 11:48
You're probably right. I'm using Resharper 2016 and Visual Studio 2015. But the point remains - you _can_ use void in an async event handler. – smoksnes Nov 23 '16 at 11:50
@smoksnes In my case, the warning is coming from [SonarLint](http://www.sonarlint.org/visualstudio/index.html). I'm not sure if this is also what the OP is using, but it's well-established that `async void` is okay for event handlers, so I should hope that if the OP is using something else, that something else uses the same logic to make an exception for event handlers. – Nov 23 '16 at 11:53
Maybe this is Xamarin related. I refactored my code using your suggestion: public event EventHandler ClickTest; void FireClickTest (int id) { if (ClickTest != null) { ClickTest (this, id); } } //in observer async void Handle_ClickTest (object sender, int e) { await DoSomething (); } However, I get the same warning. – Gerhard Schreurs Nov 23 '16 at 11:54
1

@GerhardSchreurs `int` does not derive from `EventArgs`. You should make a class that derives from `EventArgs` (call it `WidgetClickEventArgs` if you like), and store the `int` in a property in that class. Or, if the `id` merely indicates which button got clicked, just have `EventArgs e` and pass `EventArgs.Empty`, then you can check the `sender` instead of the `id`. – Nov 23 '16 at 11:54
@hvd Thank you! I'm still curious about the other comments, but deriving form EventArgs works indeed. – Gerhard Schreurs Nov 23 '16 at 12:01
This does not answer the question. Simply following the .NET convention regarding the event handlers doesn't dismiss the warning as well. – mr5 Jun 28 '19 at 05:22

score 0 · Answer 3 · answered Jul 15 '19 at 13:10

As mentioned in other answers, you can structure your event handlers correctly so that this is not raised, as the runtime will know how to correctly call the delegate. Here is some code showing you how to structure this:

Expose a public Command to your view

public ICommand CopyDeviceId { get; private set; }

Then, create a command instance with a delegate

CopyDeviceId = new Command(() => CopyDeviceIdValue(this, EventArgs.Empty));

Finally, create the asyncronous delegate

private async void CopyDeviceIdValue(object sender, EventArgs e)
{
    \\code here
}

Can I refactor code to fix "Asynchronous method should not return void"?

3 Answers3