HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Getting weather data from Open Weather Map API

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mapopengettingapifromdataweather

Problem

I'm trying to make a simple app in C# that gets the weather data from http://openweathermap.org/api (OWM).

Here attached are two classes that I use to initialize WeatherData for selected City and to download and parse XML data from OWM API. I would like to know your opinion on the structure of the classes and the way I use them, because now I'm really not sure how should it work assuming that I would like to get/print more weather data in future.

I use them as follows:

WeatherData WarsawWeather = new WeatherData("Warsaw");
WarsawWeather.CheckWeather();
System.Console.WriteLine(WarsawWeather.Temp);


Classes:

```
using System.Net;
using System.Xml;

namespace WeatherApp
{
class WeatherData
{
public WeatherData(string City)
{
city = City;
}
private string city;
private float temp;
private float tempMax;
private float tempMin;

public void CheckWeather()
{
WeatherAPI DataAPI = new WeatherAPI(City);
temp = DataAPI.GetTemp();
}

public string City { get => city; set => city = value; }
public float Temp { get => temp; set => temp = value; }
public float TempMax { get => tempMax; set => tempMax = value; }
public float TempMin { get => tempMin; set => tempMin = value; }
}
class WeatherAPI
{
public WeatherAPI(string city)
{
SetCurrentURL(city);
xmlDocument = GetXML(CurrentURL);
}

public float GetTemp()
{
XmlNode temp_node = xmlDocument.SelectSingleNode("//temperature");
XmlAttribute temp_value = temp_node.Attributes["value"];
string temp_string = temp_value.Value;
return float.Parse(temp_string);
}

private const string APIKEY = "API KEY HERE";
private string CurrentURL;
private XmlDocument xmlDocument;

private void SetCurrentURL(string location)

Solution

private float temp;
    private float tempMax;
    private float tempMin;


You should avoid abbreviating temperature. Temp is commonly used as an abbrevation for temporary and confuses here. In C# we don't usually use abbreviations. There are exceptions but they consider some well know ones like xml or html.

CurrentURL = "http://api.openweathermap.org/data/2.5/weather?q=" 
            + location + "&mode=xml&units=metric&APPID=" + APIKEY;


You use some of the new C# 6 features already like the bodyless methods => so you might want to do the same for strings. The above line will become:

_currentURL = 
    $"http://api.openweathermap.org/data/2.5/weather?q={location}" + 
    $"&mode=xml&units=metric&APPID={APIKEY}";


Using the var could make you code look much simpler.

Consider this

using (WebClient client = new WebClient())
    {
        string xmlContent = client.DownloadString(CurrentURL);
        XmlDocument xmlDocument = new XmlDocument();
        xmlDocument.LoadXml(xmlContent);
        return xmlDocument;
    }


vs that

using (var webClient = new WebClient())
    {
        var xmlContent = webClient.DownloadString(CurrentURL);
        var xmlDocument = new XmlDocument();
        xmlDocument.LoadXml(xmlContent);
        return xmlDocument;
    }


XmlDocument


There's a newer and an easer way to work with xml. Try the XDocument out which is LINQ-capable. You'll like it. See XDocument or XmlDocument

Code Snippets

private float temp;
    private float tempMax;
    private float tempMin;
CurrentURL = "http://api.openweathermap.org/data/2.5/weather?q=" 
            + location + "&mode=xml&units=metric&APPID=" + APIKEY;
_currentURL = 
    $"http://api.openweathermap.org/data/2.5/weather?q={location}" + 
    $"&mode=xml&units=metric&APPID={APIKEY}";
using (WebClient client = new WebClient())
    {
        string xmlContent = client.DownloadString(CurrentURL);
        XmlDocument xmlDocument = new XmlDocument();
        xmlDocument.LoadXml(xmlContent);
        return xmlDocument;
    }
using (var webClient = new WebClient())
    {
        var xmlContent = webClient.DownloadString(CurrentURL);
        var xmlDocument = new XmlDocument();
        xmlDocument.LoadXml(xmlContent);
        return xmlDocument;
    }

Context

StackExchange Code Review Q#150981, answer score: 4

Revisions (0)

No revisions yet.