2
\$\begingroup\$

I'm trying to make my ASP.net Web API methods asynchronous. This method is in the Business Logic layer of my N-tier. I'm waiting for the result from my 1st task and use it on the 2nd task, but I think I'm doing it in a wrong way. Can you please give me advice or feedback regarding my code?

public async Task<DTOUser> CreateUser(DTORegister source)
{
    var result = new DTOUser();

    try
    {
        var user = new DTOUser()
        {
            Active = (int)defaultNumber.One,
            Deleted = (int)defaultNumber.Zero,
            DateCreated = DateTime.Now,
        };

        result = await Task.Run(() =>_RepositoryUser.Add<DTOUser, DTOUser>(user));

        //This part is where I dont feel confident
        var login = new DTOLogin()
        {
            Email = source.Email,
            Password = source.Password,
            OwnerID = result.UserID, //using the result of my 1st task here. 
            DateCreated = DateTime.Now

        };
        //use the login object to my 2nd task
        var task2 = await Task.Run(() =>_RepositoryLogIn.Add<DTOLogin, DTOLogin>(login));

        result.IsSuccessful = true;

    }
    catch (Exception ex)
    {
        result.ErrorMsg = "BL Error - " + ex.Message;
        result.IsSuccessful = false;
    }
    return result;
}

Both _RepositoryUser.Add and _RepositoryLogIn.Add are calling the same Generic Repository method from DAL:

public TOutPut Add<TOutPut, TInput>(TInput input)
        {
            var entityObj = MappingConfiguration.MapObjects<TInput, TType>(input); //Calling my automapper method to map my Entity object and DTO Object
            _entity.AddObject(this.GetEntitySetName(typeof(TType).Name), entityObj);
            _entity.SaveChanges();
            return MappingConfiguration.MapObjects<TType, TOutPut>(entityObj); 

        }
\$\endgroup\$
  • \$\begingroup\$ Given the addition of the Add to the question do you have a SaveChangesAsync method? Not sure what DAL you are using but there is one in Entity Framework \$\endgroup\$ – dreza Jun 15 '14 at 8:28
  • \$\begingroup\$ @dreza, I don't have SaveChangesAsync because I'm only using Entity Framework version 5. \$\endgroup\$ – rcadaoas Jun 15 '14 at 9:02
  • \$\begingroup\$ ah ok then. Well seems ok I think. Here is a good stack overflow link stackoverflow.com/questions/18013523/… \$\endgroup\$ – dreza Jun 15 '14 at 9:28
4
\$\begingroup\$

To me it seems like you are trying to make a async task for the sake of making it async to fit in with.

I'm trying to make my ASP.net Web API methods asynchronous

I don't see any reason for these pieces of code to be async. They aren't doing long running tasks and aren't even hitting the db (unless Add doesn't something we don't know about of course like saves as well which is a different point of review right there).

result = await Task.Run(() =>_RepositoryUser.Add<DTOUser, DTOUser>(user));

//use the login object to my 2nd task
var task2 = await Task.Run(() =>_RepositoryLogIn.Add<DTOLogin, DTOLogin>(login));

Once you have removed these, the method becomes a truly synchronous method. As it's a pretty trivial method anyway this shouldn't be a problem and making it async only confuses the issue for no benefit.

\$\endgroup\$
  • \$\begingroup\$ Hi @dreza, so you're saying that if my method does not have any long running process then I don't need to make it asynchronous? \$\endgroup\$ – rcadaoas Jun 15 '14 at 7:59
  • 1
    \$\begingroup\$ Well my understanding in Web is that they free up thread for use to serve other requests. But if your code is so minimal that it's literally just creating objects etc then the time it takes to do this is negligent and so I can't see the benefit. Of course I'm only new to Async myself so perhaps some-one else might have more insight \$\endgroup\$ – dreza Jun 15 '14 at 8:08
  • 1
    \$\begingroup\$ This might be interesting reading for you msdn.microsoft.com/en-us/library/ee728598%28v=vs.100%29.aspx \$\endgroup\$ – dreza Jun 15 '14 at 8:09
  • 2
    \$\begingroup\$ @dreza - You're correct in that an async action will free up the thread for use with other requests. If the method isn't doing some long running IO, making it async wont improve anything. Now if all available threads are being used up, you'll see a benefit from async. \$\endgroup\$ – Smith.h.Neil Jun 16 '14 at 19:03
0
\$\begingroup\$

If that Add is calling the database, then async is warranted. However, I think you are overcomplicating it. With Async/Await, you need async/await through all the layers until you await a SaveChangesAsync or some bottom call.

Ultimately, when this async fires, it'll free up the thread for something else to do work. Also, if you are using a framework, you should do .ConfigureAwait(false) after all of your await calls or you could end up in deadlocks for users that force a sync call.

However, if you need the language/culture from the original thread later in the call, then you don't want to use ConfigureAwait(false), but you could always pull off the current values, run it, then update the thread values after the await call to fix that issue. Also, all the Async methods should have the word "Async" after them, except for the top level ones on the UI to make it clear.

This is how I'd lay it out:

public async Task<DTOUser> CreateUserAsync(DTORegister source)
{
    var result = new DTOUser();

    try
    {
        var user = new DTOUser()
        {
            Active = (int)defaultNumber.One,
            Deleted = (int)defaultNumber.Zero,
            DateCreated = DateTime.Now,
        };

        result = await _RepositoryUser.AddAsync<DTOUser, DTOUser>(user).ConfigureAwait(false);

        //This part is where I dont feel confident
        var login = new DTOLogin()
        {
            Email = source.Email,
            Password = source.Password,
            OwnerID = result.UserID, //using the result of my 1st task here. 
            DateCreated = DateTime.Now

        };

        await _RepositoryLogIn.AddAsync<DTOLogin, DTOLogin>(login).ConfigureAwait(false);

        result.IsSuccessful = true;

    }
    catch (Exception ex)
    {
        result.ErrorMsg = "BL Error - " + ex.Message;
        result.IsSuccessful = false;
    }
    return result;
}

_Repository.AddAsync, if using EF, would then await context.SaveChangesAsync().

If you wanted to go the extra mile, also have all methods take CancellationToken token = default(CancellationToken) as an optional parameter and pass that all the way down, too.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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