3
\$\begingroup\$

I'm working on a class to download some data from an external server and feed it into my data model. I'm new to both HttpClient and async methods in C#, so I'm not sure of a few things.

  1. Is my placement of new SearchResultSet() reasonable? It's a little contrived, true, but I'm trying to get an example of how async code can streamline execution.
  2. Is splitting the GetStringAsync() and await myTask statements going to cause any problems?

The HttpClient class is being passed in via the constructor to aid in unit testing.

class MySearch 
{
    private readonly HttpClient searchClient;
    private readonly Uri baseSearchUrl;

    public MySearch(HttpClient client, Uri baseUrl) {
        searchClient = client;
        baseSearchUrl = baseUrl;
    }

    public async Task<SearchResultSet> SearchAsync(string term)
    {
        var query = "?gws_rd=ssl#q=" + term;
        var searchUrl = new Uri(baseSearchUrl, query);

        try {
            var myTask = searchClient.GetStringAsync(searchUrl);
            var result = new SearchResultSet()
            {
                SearchPageUrl = searchUrl
            };

            var searchData = await myTask;

            result.Fill(searchData); //parsing HTML is off-topic for this class!

            return result;
        }
        catch (AggregateException ex)
        {
            throw ex;
        }
        catch (WebException ex)
        {
            throw ex;
        }       
        catch (Exception ex)
        {
            throw ex;
        }
    }
}
\$\endgroup\$
5
\$\begingroup\$
  • You are making your class variables readonly which is good because they won't change.

  • what I don't like is your mixing of the bracing style. Sometimes you use the Allman style, sometimes the K&R and sometimes another style.

    Usually in C# one would expect to see Allman style only. If you want to use another style, that is pretty fine as long as you stick to it.

  • the exception handling like you are doing it is superfluous and is destroying the stacktrace. Please read: is-there-a-difference-between-throw-and-throw-ex

    Because you are never handling the exceptions you could just remove the try..catch completely.

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