Weather API implementation

Question

This class uses the openweathermap API to receive information about the current weather and forecasts. I am looking for any type of advice, but primary for structure and style improvement suggestions: This method builds the the API URL:

private Uri BuildQueryURL(Interval requestType, NameValueCollection parameters)
{
    if (parameters == null) parameters = new NameValueCollection();
    string relativePath = "";
    //set query//
    switch (requestType)
    {
        case Interval.Current:
            relativePath = "weather";
            break;
        case Interval.Forecast:
            relativePath = "forecast";
            break;
    }
    //add api key//
    parameters.Add("APPID", ApiKey);
    //if needed, add unit parameter//
    if (Units != Units.Default) parameters.Add("units", Units.ToString());
    //build uri
    var builder = new UriBuilder();
    builder.Scheme = "http://";
    builder.Host = "api.openweathermap.org";
    builder.Path = Path.Combine("data/2.5", relativePath);
    builder.Query = string.Join("&", parameters.AllKeys.Select((name) => string.Format("{0}={1}", HttpUtility.UrlEncode(name), HttpUtility.UrlEncode(parameters[name]))));
    return builder.Uri;
}

The actual query is done by following method:

private RequestStatus Execute(Uri queryURI, out string queryResponse)
    {
        queryResponse = null;
        var status = RequestStatus.OK;
        try
        {
            queryResponse = _client.DownloadString (queryURI);
            //this methods parses the code parameter of the query response//
            status = CheckResponseCode(queryResponse);
        }
        catch(WebException) {
            status = RequestStatus.Failed;
        }
        return status;
    }

Here is the deserialization of the received data:

 private CurrentWeather DeserializeCurrentWeather(string queryResponse)
    {
        var currentWeather = JsonConvert.DeserializeObject<CurrentWeather>(queryResponse);
        currentWeather.Units = this.Units;
        currentWeather.Country = CountryCodeHelper.ConvertToName((string)JObject.Parse(queryResponse).SelectToken("sys.country"));
        return currentWeather;
    }

This is called by the consumer:

public RequestStatus GetWeatherByZip(int zip, string countryCode, out CurrentWeather result)
    {
        return GetWeather(new NameValueCollection() { { "zip", CombineQueryCountryCode(zip.ToString(), countryCode) } }, out result);
    }
public RequestStatus GetWeatherByCity(string city, string countryCode, out CurrentWeather result)
    {
        return GetWeather(new NameValueCollection() { { "q", CombineQueryCountryCode(city,countryCode) } }, out result);
    }
//...//
private RequestStatus GetWeather(NameValueCollection parameters, out CurrentWeather result)
    {
        string queryResponse;
        RequestStatus status = RequestStatus.OK;
        status = Execute(BuildQueryURL(Interval.Current, parameters), out queryResponse);
        result = DeserializeCurrentWeather(queryResponse);
        return status;
    }

I excluded the async methods and forecast methods. The complete source can be found at the GitHub page.

Answer 1

BuildQueryURL method

This method is in a very bad style. It's supposed to build a query url but you give it a name-value-collection that you modify it inside the method!

You should split it into two methods.

The first one should only UpdateParameters and the other one BuildQueryURL only build the url without modifying any other values and causing unexpected behaviour. I'd be really supprised if I used it and found that it actually changed the parameters too although it's name doesn't suggest anything like that.

---

Interval enum

If this is your enum then make the values exactly like the relative path so that you can just call .ToString() on it without a switch. If not then I'd use a dictionary to get rid of the switch.

---

Execute & GetWeather methods

Usually when we do somehting like this where we have an out argument and return a bool or another type of a result success indicator we name the method TryDoSomething so in your case the method should be called TryExecuteQuery and TryGetWeather