Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
 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 XDocument or XmlDocument

 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

 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

Source Link
t3chb0t
  • 44.7k
  • 9
  • 84
  • 190
 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

lang-cs

AltStyle によって変換されたページ (->オリジナル) /