Runtime: Async suffix

[joshfree]

Migrated from original issue https://github.com/dotnet/core/issues/1464

_@lukepuplett commented on Apr 19_

Should the Async suffix convention be used in 2018?

I'm so sorry for bringing this up, its not an issue as such, its debatable, but this might be the best forum for it.

Should .NET and other open-source libraries continue to append Async on awaitables?

General

My team here have begun to drop it; I always wondered whether this might happen but thought it premature until I read this.

https://docs.particular.net/nservicebus/upgrades/5to6/async-suffix

What do we feel is appropriate guidance in 2018?

Thanks

_@deerchao commented on May 29_
Never used the Async suffix in my code.

_@MarcoTheFirst commented 11 days ago_
I totally agree. There are enough indications that a method is async:

1. Return type is Task / Task<>
2. Code completion presents an awaitable hint
3. the IDE will warn you by underlining in green
4. Compiler will present a warning when task is not awaited.

Using the Async suffix only makes sense if your API offers the same functionality in both flavors:

GetObject()
GetObjectAsync()

For any other use case, using the Async-suffix is as silly as adding a Lazy-suffix to any Lazy<> property:

public Lazy<Customer> CustomerLazy.

If you wouldn't do that, then don't use the Async suffix :-)

_@lukepuplett commented 6 days ago_
Thanks for your answer.

For accuracy, number one isn't quite right, the compiler works by looking for an awaitable type, which is any type that has TaskAwaiter GetAwaiter() on it.

[jnm2]

It's only ever been around because you can't overload by return type in languages like C#. If you can rule out the possibility of adding synchronous versions of the methods in the future, I see scenarios where I can't think of any reason to use the suffix. It depends on context.
Another data point is C#'s async Main which isn't MainAsync because there's no need to maintain a synchronous version of it.

[NickCraver]

Let's say you are reviewing a PR. Let's say they add this line of code:
```c#
var result = GetResult();

Is it sync or async? Did someone forget to `await` the result? What's the return type? None of these things are apparent. Now let's compare:
```c#
var result = GetResultAsync();

...that's obvious. And it cost you nothing to make it obvious. No suffix may have no issues in specific scenarios where you know a lot. Having a suffix always has no issues. It's only a positive.

As someone who reviews code a lot, people can pry the Async suffix out of my cold, dead fingers. And they better hope rigor mortis hasn't set in when they try.

[vcsjones]

Should .NET and other open-source libraries continue to append Async on awaitables?

Yes.

I look forward to this question being asked again in 2019.

On a more serious note, @NickCraver more or less hit the nail on the head. Compiler warnings are nice, but code should be readable and expressive without depending on tooling to know exactly what is going on. Nick's example is great, but it is even more compelling for me when you have Task and no return value at all.

A code base is unlikely to ever be 100% async. That would be silly, and indeed async is not the right answer all the time. Because we'll always have async and sync code, the clearest way currently to express clarity is with a suffix.

You are of course free to do whatever is you feel is best for your project. Octokit is an example of a project that does not use the suffix.

I think the best option is consistency. For CoreFX, that means using async suffixes for the foreseeable future.

[daveaglick]

Async has such profound implications to control flow, performance, etc. that an extra visual cue at the call site is both warranted and appreciated, IMO

[khalidabuhakmeh]

Regardless of the "right" choice, wouldn't this be a Godzilla-sized breaking change? Maybe if we bumped the version of .NET Core to 11 then it may be ok... but still.

[daveaglick]

Maybe if we bumped the version of .NET Core to 11 then it may be ok

Nah, that won’t work. I’m pretty sure .NET Core 11 comes right after .NET Core 2 in the latest revised versioning scheme.

[SimonCropp]

@NickCraver isnt var result = GetResult(); a misleading representation? wouldn't it usually be var result = await GetResult(); and hence "this is async" is obvious without the Async Suffix?

[MarkusAmshove]

@SimonCropp that is exactly the point :-)

You're seeing var result = GetResult(); while reviewing code (without an IDE) and you can't be sure if it is async (missing await) or if it is not async.

var result = GetResult(); is legal code, where result would be of type Task when not awaited.

If it was var result = GetResultAsync() it would be a good hint for the reviewer that something is wrong.

[SimonCropp]

@MarkOsborneMS my point is GetResult implies there is some result to be used. so it would not be a simple Task. it would be a Task<T> and to use that result then it needs to be awaited and hence wont compile without the await.

if GetResult(); only returns a Task, and you do want to force it to be awaited when it has no return value, then that would mean var result = GetResult(); (missing an await) has no use of result, which could easily be caught by treating un-used variables as an error.

[MarcoTheFirst]

@NickCraver: While I agree with your point, I‘d like to point out once again that your argument is true for any piece of code: The meaning of

var result = GetResult()

is NEVER clear since there‘s absolutely no type information to see. So this example is not specific to Async. What if the return type were Lazy<>, would you expect the method to be called GetResultLazy? If it were an int, would you call it GetResultInt ?

[benaadams]

So this example is not specific to Async. What if the return type were Lazy<>, would you expect the method to be called GetResultLazy? If it were an int, would you call it GetResultInt ?

It is because neither Lazy nor Int change the flow control; its fine to ignore the result of awaiting a Task<T> for example; but accidentally not awaiting it leads to entirely different problems and introduces concurrency and race conditions.

[MarcoTheFirst]

@benaadams not awaiting a task will return the Task itself instead of the result type. therefore it is impossible to accidentally confuse the two. Your strongly typed code simply won‘t build, so I still disagree with the Async suffix. And since a suffix is just sugar coating anyway it‘s not reliable. Programmers make mistakes, compilers usually dont‘t.

[NickCraver]

Your strongly typed code simply won‘t build, so I still disagree with the Async suffix.

This is simply false. Let's take my original example and put it in a simple program:
```c#
void Main()
{
var result = GetResult();
}

async Task GetResult()
{
await Task.Delay(4000);
return 5;
}
```

This compiles just fine. And it exits immediately, terminating whatever task was in progress. It'd be a shame if that was updating a bank balance or something...

Or let's take other real-life examples. Let's say it was fetching a result from SQL. Fine, right? We didn't want the result anyway. But wait, SQL connections with concurrent usage (by default - so for most apps) aren't safe. You have 2 consumers of the TDS stream. That yields random results from both sides. Because the first one wasn't awaited, both are racing and competing on something not thread safe. This is just one of thousands such examples. And I've seen them happen (as have many here), they're unfortunately far from theoretical.

[MarkusAmshove]

What @MarcoTheFirst means (I think) is the following case:

void Method()
{
    var result = GetResult();
    int x = result + 10; // Compile error, Task<int> + int doesn't work
}

However, I find the other way around more dangerous:

db.SaveCustomer(c);
db.SaveOrder(o);
….

[Clockwork-Muse]

@MarkusAmshove - yeah, but ignoring a return value is (almost always) a programmer error, and the compiler should be squawking at you for that.

[MarkusAmshove]

Then the Roslyn team needs to do something, because this doesn't yield an error or warning:

static void Main(string[] 
{
    Save();

}
public static Task Save()
{
     return Task.CompletedTask;
}

[Clockwork-Muse]

....I know it doesn't _now_, but in my opinion it should.

[satjinder]

Not sure if it would result in other kinds of problems or confusions, but I think the life could have been easier if await is by default for async methods and you could choose something like "getTask DoSomething()".

i.e. Explicit opt-out of await with default opt-in.

[stephentoub]

I don't believe there's any further action item required on this issue. Given the lack of discussion on the naming issue for several months, I'm going to close this out. Thanks for the good discussion.