patterncsharpMinor
Getting weather data from Open Weather Map API
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:
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)
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;
}XmlDocumentThere'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 XmlDocumentCode 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.