CODEWITHSUNDEEP
c#asynchronousasp.net-web-api
AntDC
AntDC

Reputation: 1917

Can I call a 'normal' static method from an Async API Controller method

relatively new to WebAPI and Async so please bear with me....

I have a WebAPI application with a number of operations in it. Essentially, these call SQL Server stored procedures.

It has been working fine, but I am looking to make it more efficient and robust by converting the methods to be Async.

Let me show one methed - as they are all similar....

OLD Version

[System.Web.Http.HttpGet]
public A_Class MyAPIMethod(Guid b)
{
    using (SqlConnection DB = new SqlConnection(this.dbConnString))
    {
        return MyStaticHelper.A_Static_Method(DB, b);
    }
}

New Version (Async I think & hope)

[System.Web.Http.HttpGet]
public async Task<A_Class> MyAPIMethodAsync(Guid b)
{
    var db = new SqlConnection(this.dbConnString);
    try
    {
        var Result = await Task.Run(() => MyStaticHelper.A_Static_Method(db, b));
        return Result;
    }
    finally
    {
        db.Dispose();       
    }
}

I think that bit is OK - I am just not sure what, if anything I need to do to my static helper method. Do I need to convert that to be Async? I have made calls to this and it all seems to work OK - I could just do with a sanity check please. Any advise appreciated....

Static Helper Method....

public static A_Class A_Static_Method(SqlConnection dbConn, Guid A_Param)
{
    SqlDataReader reader = null;
    try
    {
        try
        {
            if (dbConn.State != ConnectionState.Closed) dbConn.Close();

            using (var cmd = new SqlCommand("MyStoredProc", dbConn))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.Add("@sp_Param", SqlDbType.UniqueIdentifier).Value = A_PAram

                dbConn.Open();
                reader = cmd.ExecuteReader();

                var A_Class = StaticFunctionToGetInstanceOfClassFromResults( reader );
                A_Class.rc = 1;
                return A_Class 
            }
        }
        catch (Exception)
        {
            //server error
            return new A_Class(A_ClassError.beApiError);
        }
    }
    finally
    {
        if (reader != null) reader.Close();
        dbConn.Close();
    }            
}

Upvotes: 0

Views: 2368

Answers (2)

Jon Hanna
Jon Hanna

Reputation: 113382

var Result = await Task.Run(() => MyStaticHelper.A_Static_Method(db, b));

Passes work to a new thread. Releases the old thread. Waits on result in new thread. Now a thread can carry on where this left off.

This could be useful if you were running several such tasks simultaneously, but you're spending effort to relieve a thread by having a thread do stuff. That's all cost and no gain.

Where async wins is:

  1. You have more than one async operation at the same time.
  2. You have an async operation that is using asynchronous I/O so that the calling thread is released entirely.

The second is the more important, especially in web terms.

Let's consider your "helper" method first. There are calls in that that have truly async equivalents, and so we can create a truly async version:

public static async Task<A_Class> AStaticMethodAsync(SqlConnection dbConn, Guid A_Param)
{
  try
  {
    if (dbConn.State != ConnectionState.Closed) dbConn.Close();

    using (var cmd = new SqlCommand("MyStoredProc", dbConn))
    {
      cmd.CommandType = CommandType.StoredProcedure;
      cmd.Parameters.Add("@sp_Param", SqlDbType.UniqueIdentifier).Value = A_PAram

      await dbConn.OpenAsync();
      using(SqlDataReader reader = await cmd.ExecuteReader())
      {
        var A_Class = StaticFunctionToGetInstanceOfClassFromResults( reader );
        A_Class.rc = 1;
        return A_Class
      }
    }
  }
  catch (Exception)
  {
    //server error
    // Why are you wrapping an exception instead of just passing it up the stack? This is weird.
    return new A_Class(A_ClassError.beApiError);
  }
}

Two notes:

  1. I replaced your use of try…finally around the SqlDataReader with a more normal using.
  2. Presumably StaticFunctionToGetInstanceOfClassFromResults calls Read() on the datareader and then builds an object based on that. You could add an async version that calls await ReadAsync() and then use var A_Class = await StaticFunctionToGetInstanceOfClassFromResultsAsync(reader) here for even better asynchronous behaviour.

Now that we've an async method, your controller can be:

[System.Web.Http.HttpGet]
public async Task<A_Class> MyAPIMethodAsync(Guid b)
{
  using (SqlConnection DB = new SqlConnection(this.dbConnString))
  {
    return await MyStaticHelper.AStaticMethodAsync(DB, b);
  }
}

And it truly benefits from asynchronous behaviour.

Upvotes: 1

Stephen Cleary
Stephen Cleary

Reputation: 457302

No, the approach is wrong. In general, you should avoid Task.Run (and any other method that queues work to the thread pool) on ASP.NET.

Instead of starting from the controller and working "down", you should start at the lowest level and work up. That is, first examine your static method and determine if there are any naturally-asynchronous operations. These are generally I/O. Two jump immediately out to me: opening the database connection and retrieving results from a query (and it's likely that StaticFunctionToGetInstanceOfClassFromResults has more). You should call those with await first, and then allow async to grow naturally towards your controller (the compiler will guide you).

Also, as @StephenBrickner commented, you might want to take a step back and determine whether async will help you. I have an article on async ASP.NET that covers the primary considerations. In particular, if your backend won't scale (e.g., if it's a single SQL server instance, not something like Azure SQL), then there's (usually) no point in making your web server scale.

Upvotes: 1

Related Questions