CODEWITHSUNDEEP
c#async-await
Anton Sizikov
Anton Sizikov

Reputation: 9230

Distinguish between void and task returning method

I'm working on a class which has a method with Action<T> parameter:

public void RegisterCallback<T> (Action<T> callback);

This works good enough.

instance.RegisterCallback<string>(Callback); //method group
instance.RegisterCallback<string>(t => {}); //lambda

Now I want to create an overload for this method which accepts async method. So that I can use it with task returning callbacks and treat them in a different way.

instance.RegisterCallback<string>(AsyncCallback); //method group with async

Where AsyncCallback is

    private Task AsyncCallback(string s)
    {
        return Task.Delay(0);
    }

The naive approach was to have a method like this one:

void RegisterCallback<T>(Func<T, Type> callback);

It has some problems though:

//1
instance.RegisterCallback<string>(async t => await Task.Delay(0));
//2
instance.RegisterCallback<string>(AsyncCallback); //method group with async

the first one gets resolved to Action<T> callback overload and the second fails to compile because of ambiguous invocation.

Well, that makes sense and I'm ok about this interface:

 void RegisterAsyncCallback<T>(Func<T, Task> callback);

However these two calls are compiled with no problems:

   instance.RegisterCallback<string>(async t => await Task.Delay(0));
   instance.RegisterAsyncCallback<string>(async t => await Task.Delay(0));

Is there a way to design this public api so that a user will only use void callbacks with one method and task returning with another.

Perhaps someone can point me to the existing api where similar problem was solved?

The full code can be found here.

Upvotes: 3

Views: 127

Answers (2)

David Pine
David Pine

Reputation: 24525

Is there a way to design this public api so that a user will only use void callbacks with one method and task returning with another.

No, not really. The issue is the Action<T> delegate. It allows for a large number of various lambda expressions as you have already discovered. The one major issue that you will run into with the RegisterCallback(Action<T> callback) - if anyone uses it like this:

instance.RegisterCallback<string>(async _ => await Task.Delay(0));

They are creating an async void and this is a huge problem. You had the right idea with the Func<T, Task>. Also, I like your idea to rename the function and adding "Async" seems appropriate, RegisterAsyncCallback(Func<string, Task> callback).

Upvotes: 2

Stephen Cleary
Stephen Cleary

Reputation: 456437

There's no good solution here; you'll either have the ambiguous call error with method overloads, or you'll have a whole new API.

Personally, I would add the Func<T, Task> overload and take the breaking change, no longer allowing method overloads. Alternatively, you can create both overloads on the new method (RegisterCallbackEx?) and mark the old one as Obsolete - this would allow older code to compile but encourage devs to change the calls.

Perhaps someone can point me to the existing api where similar problem was solved?

Well, I can give you an example where a similar problem wasn't solved. :)

Task.Factory.StartNew is commonly used to queue work to the thread pool. It's Action-based, and I believe it works with method groups.

When the .NET team wanted to add async support, they introduced a new API instead: Task.Run, which is overloaded for Action and Func<Task>.

(Note that in this case, Task.Run technically supports only a subset of StartNew uses, and StartNew was not marked Obsolete).

They considered their options, but this was the best they could come up with. Note that the following problems still exist, and developers ask questions about both of these points rather regularly here on SO:

  • Task.Run does not support method groups.
  • StartNew will compile just fine with async lambdas, but will behave in surprising ways (due to async void).

Since they were changing the .NET BCL, backwards compatibility was paramount. For a personal library, I prefer safer/cleaner APIs over strict backwards compatibility, so I would make the other choice in your situation (i.e., just add a Func<T, Task> overload as part of a major version upgrade).

Upvotes: 3

Related Questions