1

We have a legacy method with return type of async Task<>. Its current implementation needs to query the database thus the need for asynchronous operation. But we're changing it to just use in-memory objects so basically we don't need to have an asynchronous operation in the implementation anymore but I don't want to change the method's signature to not break any callers of the method.

Now, should I wrap the implementation in an await Task.FromResult or should I just not use any await and just suppress the warning?

Jan Paolo Go
  • 5,842
  • 4
  • 22
  • 50
  • I was asking myself this exact question this morning. I'm a little surprised there is even a warning for this. If performance is a concern, you would want to avoid the unnecessary `await` but otherwise I don't see a huge problem with doing it that way – Joe Phillips Feb 28 '19 at 23:06
  • Hi Paulo, I would suggest, to use the `await Task.FromResult` because when doing async operations it's async all the way (which is good). Still there will not be any impact because the task will immediately return with the response you want to. In this way you are not modifying the current abstractions (or the method signatures) and "if" there are changes to the approach taken such as query the database or from another service, you just need to implement that part. – Cheranga Feb 28 '19 at 23:14
  • 2
    @Cheranga "async all the way" means that all your call chain should return tasks, not that you have to use the `async` keyword – Kevin Gosse Feb 28 '19 at 23:16
  • I correct my comment, there *is* a duplicate. Remove `async` and just return `Task.FromResult` – Camilo Terevinto Feb 28 '19 at 23:17
  • @CamiloTerevinto A perfectly reasonable reason is for consistency – Joe Phillips Mar 01 '19 at 15:24

1 Answers1

4

The generally accepted way, and the way largely used in the Microsoft docs, is to remove the async keyword and return Task.FromResult. Use Task.CompletedTask if the method needs no return type (aside from Task, obviously).

RecyclingBen
  • 310
  • 3
  • 9
  • So it's safe to assume that removing the `async` keyword won't break any callers of the method? – Jan Paolo Go Feb 28 '19 at 23:11
  • 1
    @PaoloGo Yes, it's safe. `async` marks the current method, the client cannot know if the method was marked with `async` or not – Camilo Terevinto Feb 28 '19 at 23:12
  • Yes. Behind the scenes, async would be functionally equivalent to returning a Task in this case. – RecyclingBen Feb 28 '19 at 23:13
  • I see. in short `async` is just a compiler thing? – Jan Paolo Go Feb 28 '19 at 23:14
  • 2
    There's one behavior change to be aware of: if you throw an exception in an `async` method, it will be trapped and a faulted task will be returned. If you remove the `async` keyword, then the exception will propagate like an "ordinary" method. It's usually not a concern, but that's something to keep in mind – Kevin Gosse Feb 28 '19 at 23:14
  • 2
    @PaoloGo: Yes, it's a compiler thing. `async` signals to the compiler that `await` is allowed (indeed, expected) in the method, and causes the compiler to rewrite the method body into an asynchronous workflow. It does nothing to the *signature* of the method. – Eric Lippert Feb 28 '19 at 23:18
  • Got it! Thanks guys! We'll remove the `async` since it won't change the signature of the method. :) – Jan Paolo Go Feb 28 '19 at 23:25