Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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); 

        }
share|improve this question
    
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 –  dreza Jun 15 at 8:28
    
@dreza, I don't have SaveChangesAsync because I'm only using Entity Framework version 5. –  rcadaoas Jun 15 at 9:02
    
ah ok then. Well seems ok I think. Here is a good stack overflow link stackoverflow.com/questions/18013523/… –  dreza Jun 15 at 9:28
add comment

1 Answer

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.

share|improve this answer
    
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? –  rcadaoas Jun 15 at 7:59
    
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 –  dreza Jun 15 at 8:08
1  
This might be interesting reading for you msdn.microsoft.com/en-us/library/ee728598%28v=vs.100%29.aspx –  dreza Jun 15 at 8:09
1  
@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. –  Smith.h.Neil Jun 16 at 19:03
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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