3
\$\begingroup\$

The following code compiles correctly:

    public ContextStatus Commit(string userName)
    {
        var status = new ContextStatus();

        var task = Task.Run(async () => { status = await CommitAsync(userName, new CancellationToken()); });
        task.Wait();

        return status;
    }

    public async Task<ContextStatus> CommitAsync(string userName, CancellationToken ct)
    {
        var status = new ContextStatus();

        try
        {
            if (this.Database.Connection.State == ConnectionState.Closed)
            {
                await this.Database.Connection.OpenAsync(ct).ConfigureAwait(false);
            }

            using (var command = this.Database.Connection.CreateCommand())
            {
                SqlParameter param = new SqlParameter()
                {
                    ParameterName = "@user",
                    SqlDbType = SqlDbType.VarChar,
                    Size = 127,
                    Value = userName
                };

                command.CommandText = "setcontextinfo";
                command.CommandType = CommandType.StoredProcedure;
                command.Parameters.Add(param);

                await command.ExecuteNonQueryAsync(ct).ConfigureAwait(false);
            }

            await base.SaveChangesAsync(ct).ConfigureAwait(false);
        }
        catch (DbEntityValidationException dbEx)
        {
            status.SetErrors(dbEx.EntityValidationErrors);
        }
        catch (DbUpdateException duEx)
        {
            status.SetErrors(duEx);
            if (!status.DatabaseErrors.Any())
                throw;
        }

        return status;
    }

Questions:

  1. Will this CommitAsync method execute synchronously in the Commit method?
  2. Is it best practive to pass new CancellationToken() in the Commit method?
  3. Will the base.SaveChangesAsync() call in CommitAsync() close the connection if it was opened beforehand?
\$\endgroup\$
2
  • \$\begingroup\$ Normally you would have a regular method and an Async method that wrap/call your regular method but you seem to be doing the oposite. If I were to use your code a call Commit, it turn out to be Async. While calling CommitAsync turns out to be non async? \$\endgroup\$
    – Tien Dinh
    Commented Dec 27, 2014 at 18:00
  • 1
    \$\begingroup\$ @TienDinh That's not a very good approach. You shouldn't have methods that pretend to be asynchronous, but actually are synchronous, just on another thread. \$\endgroup\$
    – svick
    Commented Feb 17, 2015 at 12:09

1 Answer 1

3
\$\begingroup\$

Will this CommitAsync method execute synchronously in the Commit method?

I wouldn't phrase it that way, CommitAsync always executes asynchronously, but Commit wait for it to complete synchronously.

Is it best practive to pass new CancellationToken() in the Commit method?

It's better to use CancellationToken.None, to make the meaning clearer. Though functionally, the two are equivalent.

Will the base.SaveChangesAsync() call in CommitAsync() close the connection if it was opened beforehand?

I don't think it will, a single connection can be used for multiple commands.

In general, you should create a new connection for each separate operation (which may comprise more than one command) you do and then dispose the connection, which closes it. This won't hurt efficiency, because connections are actually pooled, so closing the connection object means that the actual connection can be reused by another operation.


var status = new ContextStatus();

var task = Task.Run(async () => { status = await CommitAsync(userName, new CancellationToken()); });
task.Wait();

return status;

This is unnecessarily verbose and complicated. Instead, I would write it like this:

return Task.Run(() => CommitAsync(userName, new CancellationToken())).Result;

CommitAsync returns Task<ContextStatus>, so Task.Run() will also return that type. The Task's Result will give you the ContextStatus, synchronously waiting for the Task to complete before returning.


SqlParameter param = new SqlParameter()

Why not use var here too? The type of the variable is clear, so specifying the type just makes the code more verbose.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.