Problem
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.
- 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. - Is splitting the
GetStringAsync()
andawait 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;
}
}
}
Solution
-
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.