Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

On ASP.NET MVC, I try to write an async Controller action with the old asynchronous programming model (actually, it is the current one, new one is still a CTP).

Here, I am trying to run 4 operations in parallel and it worked great. Here is the complete code:

public class SampleController : AsyncController {

    public void IndexAsync() {

        AsyncManager.OutstandingOperations.Increment(4);

        var task1 = Task<string>.Factory.StartNew(() => {

            return GetReponse1();
        });
        var task2 = Task<string>.Factory.StartNew(() => {

            return GetResponse2();
        });
        var task3 = Task<string>.Factory.StartNew(() => {

            return GetResponse3();
        });
        var task4 = Task<string>.Factory.StartNew(() => {

            return GetResponse4();
        });

        task1.ContinueWith(t => {

            AsyncManager.Parameters["headers1"] = t.Result;
            AsyncManager.OutstandingOperations.Decrement();
        });

        task2.ContinueWith(t => {

            AsyncManager.Parameters["headers2"] = t.Result;
            AsyncManager.OutstandingOperations.Decrement();
        });

        task3.ContinueWith(t => {

            AsyncManager.Parameters["headers3"] = t.Result;
            AsyncManager.OutstandingOperations.Decrement();
        });

        task4.ContinueWith(t => {

            AsyncManager.Parameters["headers4"] = t.Result;
            AsyncManager.OutstandingOperations.Decrement();
        });

        task3.ContinueWith(t => {

            AsyncManager.OutstandingOperations.Decrement();

        }, TaskContinuationOptions.OnlyOnFaulted);
    }

    public ActionResult IndexCompleted(string headers1, string headers2, string headers3, string headers4) {

        ViewBag.Headers = string.Join("<br/><br/>", headers1, headers2, headers3, headers4);

        return View();
    }

    public ActionResult Index2() {

        ViewBag.Headers = string.Join("<br/><br/>", GetReponse1(), GetResponse2(), GetResponse3(), GetResponse4());

        return View();
    }

    #region helpers

    string GetReponse1() {

        var req = (HttpWebRequest)WebRequest.Create("http://www.twitter.com");
        req.Method = "HEAD";

        var resp = (HttpWebResponse)req.GetResponse();

        return FormatHeaders(resp.Headers);
    }

    string GetResponse2() {

        var req2 = (HttpWebRequest)WebRequest.Create("http://stackoverflow.com");
        req2.Method = "HEAD";

        var resp2 = (HttpWebResponse)req2.GetResponse();

        return FormatHeaders(resp2.Headers);
    }

    string GetResponse3() {

        var req = (HttpWebRequest)WebRequest.Create("http://google.com");
        req.Method = "HEAD";

        var resp = (HttpWebResponse)req.GetResponse();

        return FormatHeaders(resp.Headers);
    }

    string GetResponse4() {

        var req = (HttpWebRequest)WebRequest.Create("http://github.com");
        req.Method = "HEAD";

        var resp = (HttpWebResponse)req.GetResponse();

        return FormatHeaders(resp.Headers);
    }

    private static string FormatHeaders(WebHeaderCollection headers) {

        var headerStrings = from header in headers.Keys.Cast<string>()
                            select string.Format("{0}: {1}", header, headers[header]);

        return string.Join("<br />", headerStrings.ToArray());
    }

    #endregion

}

I have also Index2 method here which is synchronous and does the same thing.

I compare two operation execution times and there is major difference (approx. 2 seconds)

But I think I am missing lots of things here (exception handling, timeouts, etc). I only implement the exception handling on task3 but I don't think it is the right way of doing that.

What is your opinion on this code? How can it be improved?

share|improve this question

1 Answer

up vote 1 down vote accepted

I would certainly include exception handling in the process. I normally use try catch statements. This at least allows me to catch the exceptions and return a more meaningful and informative message vs letting a method create a silent exception that may not be returned.

Example within the helper methods:

        string GetReponse1() {
        try
        {
            var req = (HttpWebRequest)WebRequest.Create("http://www.twitter.com");
            req.Method = "HEAD";
            var resp = (HttpWebResponse)req.GetResponse();
            return FormatHeaders(resp.Headers);
        }
        catch (Exception ex)
        c{
        throw new Exception("There was a problem creating web request" + ex.InnerException);
        { 
      }

Since you are making async calls I would look to handle timeouts or prolonged responses. You could do this by creating a timer (stopwatch class). I would allow this to be configurable by the user or within a config file but I would include a stopwatch object in the method that will cancel the async call and return a timeout response once "x" (10 seconds as an example) time was reached. You could include this with the helper methods so that the call will expire after a certain time.

Additional examples of try catch statements or the stopwatch class may be found at:

Stopwatch class MSDN

Try Catch Statements

share|improve this answer

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.