2

I came across the following method in a tutorial;

    private async Task<ClaimsIdentity> GetClaimsIdentity(string userName, string password)
    {
        if (string.IsNullOrEmpty(userName) || string.IsNullOrEmpty(password))
            return await Task.FromResult<ClaimsIdentity>(null);

        // get the user to verifty
        var userToVerify = await _userManager.FindByNameAsync(userName);

        if (userToVerify == null) return await Task.FromResult<ClaimsIdentity>(null);

        // check the credentials
        if (await _userManager.CheckPasswordAsync(userToVerify, password))
        {
            return await Task.FromResult(_jwtFactory.GenerateClaimsIdentity(userName, userToVerify.Id));
        }

        // Credentials are invalid, or account doesn't exist
        return await Task.FromResult<ClaimsIdentity>(null);
    }

The author always uses await Task.FromResult<ClaimsIdentity>(...) even when returning null. I'm no expert in the Task-await pattern and would have written the method something like this;

    private async Task<ClaimsIdentity> GetClaimsIdentity(string userName, string password)
    {
        if (string.IsNullOrEmpty(userName) || string.IsNullOrEmpty(password))
            return null;

        // get the user to verifty
        var userToVerify = await _userManager.FindByNameAsync(userName);

        if (userToVerify == null) return null;

        // check the credentials
        if (await _userManager.CheckPasswordAsync(userToVerify, password))
        {
            return _jwtFactory.GenerateClaimsIdentity(userName, userToVerify.Id);
        }

        // Credentials are invalid, or account doesn't exist
        return null;
    }

Both compile. What is the difference (if any) between these two methods? Is there anything to be gained by using await Task.FromResult<ClaimsIdentity>(null) in this manner?

mark_h
  • 5,233
  • 4
  • 36
  • 52
  • 2
    That is a perfect example of wasted printable characters and redundant code. not to mention possibly another `IAsyncStateMachine` Implementation – TheGeneral Feb 10 '20 at 08:27
  • 1
    Did you ask same question from the author of the code? – Fabio Feb 10 '20 at 08:29
  • 6
    This constructs a new task around the result that has already completed, so no, there is no point in using Task.FromResult in this case. This method is for use in non-async methods that return tasks. – Lasse V. Karlsen Feb 10 '20 at 08:30
  • @Fabio I did not ask the author any questions about this. Sadly I no longer remember where I got the snippet from. I was concerned that I had misunderstood how async-await worked in some fundamental way but looking at the other comments it seems that my suspicions that wrapping the nulls is not required were correct. – mark_h Feb 10 '20 at 08:36
  • 2
    You can check out [the difference](https://sharplab.io/#v2:CYLg1APgAgTAjAWAFBQAwAIpwKwG5lQDMmM6AwugN7Lq2bFQAcmAbADwDyARgFYCmAYwAuAPgBKfYAFcAdsD4yhACgCUNOtSR1tmAOyYAnKwB0AMQBOAewC2EgM5SANkM69BopTKeOV+LXQBfdR16TGYodm5+YREAWQBLGXjrAENHVWCqTO0ofS9HRz9tIP9aIkwAFnRY1XRMzWLkAKA) here – TheGeneral Feb 10 '20 at 08:39
  • 1
    `await Task.FromResult` does nothing except waste CPU cycles. PS: apparently the source of this code is https://fullstackmark.com/post/13/jwt-authentication-with-aspnet-core-2-web-api-angular-5-net-core-identity-and-facebook-login and it was probably created by automatic refactoring of a non-async method into an async method, as the author uses normal `return`s from async methods elsewhere. – Anton Tykhyy Feb 10 '20 at 09:18
  • 1
    Take a look at similar question and relevant answer [here](https://stackoverflow.com/questions/19568280/what-is-the-use-for-task-fromresulttresult-in-c-sharp) – Sayedeh Mahshid Fatemi Feb 10 '20 at 10:23

1 Answers1

3

According to the best stackoverflow answer I've found about Task.FromResult: https://stackoverflow.com/a/19696462/6440521

The usage of Task.FromResult is appropriate only in the context of synchronous methods and mocking. So using it in an async method when you just want to return a result is redundant - gives you no additional benefits, also AsyncGuidance does not say anything about using Task.FromResult in an async method: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md

So AFAIK using Task.FromResult in an async method is unnecessary, bloats your code and gives you no real benefits.

  • What makes that answer the 'best stackoverflow answer'? – ColinM Feb 10 '20 at 09:48
  • 2
    The author of this answer is Stephen Cleary –  Feb 10 '20 at 09:49
  • While Stephen holds a great deal of knowledge, marking it as the 'best stackoverflow answer' is a bold statement. There are thousands if not hundreds of thousands of answers on stackoverflow. – ColinM Feb 10 '20 at 09:53
  • 3
    @BART Thanks for the answer, you do say its the best answer *you've* found relating to this specific issue, not that this is the definitively best answer on all of Stackoverflow. You also give due credit to others and provided a link to the original source. – mark_h Feb 10 '20 at 10:21
  • @mark_h please note my comments were regarding a previous version of the answer. – ColinM Feb 10 '20 at 11:16
  • [`await Task.FromResult(x)` is exactly the same as `x` except the code is both less efficient and less readable](https://twitter.com/aSteveCleary/status/1155827537232220161). – Stephen Cleary Feb 12 '20 at 02:55