3
\$\begingroup\$

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?

\$\endgroup\$
1
\$\begingroup\$

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

\$\endgroup\$
2
\$\begingroup\$

There is a lot of code repetition. I would create a custom class that inherits from Task and make it generic enough so you can have an array of your custom class and just loop through that array to do what you need to do.

class TaskController<T> : Task<T>
{
    public TaskController(AsyncManager asyncManager, string asyncManagerParameter, Func<T> callback) : base(callback)
    {
        this.ContinueWith(t =>
        {
            //just for demonstration
            Console.WriteLine(t.Result);

            // Do some work with your asyncManager
            asyncManager.Parameters[asyncManagerParameter] = t.Result.ToString();
        });
    }
}

I would also have GetResponse take a string uri as parameter instead of making a new GetResponse for each url that you want to use.

public static string GetResponse(string uri)
{
    HttpWebRequest req = (HttpWebRequest)WebRequest.Create(uri);
    req.Method = "HEAD";

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

    return resp.Headers.ToString(); // just changed this for demonstration purposes
}

and finally I would use it like this

AsyncManager asyncManager = new AsyncManager();

TaskController<string>[] taskControllers = {
    new TaskController<string>(asyncManager, "header1", () => GetResponse("http://google.com")),
    new TaskController<string>(asyncManager, "header3", () => GetResponse("http://www.twitter.com"))
};

foreach (TaskController<string> tc in taskControllers)
{
    tc.Start();
}

It is much less code, you can reuse it later, you can very easily add a new TaskController if you want to work with another URL, and it's a lot more readable.

To use this you will need to just edit this.ContinueWith inside TaskController and have it do whatever you want (decrement, etc)

\$\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.