I would like feedback on my Local weather app. This site simply gets the local weather. I refactored it from jQuery to Typescript to try to learn more about Typescript.
Here is a demo: https://capozzic1.github.io/local_weather_app_v2/
Here is the index.ts:
class LocalWeather {
private location: string;
private temp: number;
private description: string;
private humidity: number;
private icon: string;
private celsius: number;
private lat: number;
private long: number;
constructor() {
}
public async getCoords(): Promise<any> {
let coords = await fetch('https://geoip.nekudo.com/api').then((response) => response.json());
this.location = coords.city;
//console.log(coords)
this.lat = coords.location.latitude;
this.long = coords.location.longitude;
let weatherURL = `https://cors-anywhere.herokuapp.com/http://api.openweathermap.org/data/2.5/weather?lat=${this.lat}&lon=${this.long}&units=imperial&APPID=ef73411c829a4563b61b64e76cb72976`;
this.getWeatherData(weatherURL);
}
private async getWeatherData(url: string): Promise<any> {
let weatherData = await fetch(url).then((result => result.json()));
this.icon = weatherData.weather[0].icon;
this.temp = weatherData.main.temp;
this.description = weatherData.weather[0].description;
this.humidity = weatherData.main.humidity;
this.handleBackgroundImg();
this.displayWeather();
}
private handleBackgroundImg(): void {
let weather = document.querySelector(".weather") as HTMLElement;
switch (this.icon) {
case "01d":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img923/5914/C8hAMP.jpg")';
break;
case "01n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img924/3455/slHReo.jpg")';
break;
case "02d":
case "03d":
case "04d":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img922/8471/LF0cGZ.jpg")';
break;
case "02n":
case "03n":
case "04n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img923/7048/Ot7l6k.jpg")';
break;
case "09d":
case "10d":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img921/375/RMia6h.jpg")';
break;
case "09n":
case "10n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img922/1227/Lf4qc2.jpg")';
break;
case "11d":
case "11n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img924/2258/h4eNcE.jpg")';
break;
case "13d":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img922/3753/cEf7xg.jpg")';
break;
case "13n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img923/1196/G2MDy6.jpg")';
break;
case "50d":
case "50n":
weather.style.backgroundImage = 'url("http://imageshack.com/a/img921/8166/H0mO7r.jpg")';
break;
}
}
private displayWeather(): void {
let locationSelect = document.querySelector(".location") as HTMLElement;
let tempSelect = document.querySelector(".temp") as HTMLElement;
let newI = document.createElement('i');
let descSelect = document.querySelector(".desc") as HTMLElement;
let humidSelect = document.querySelector(".humid") as HTMLElement;
newI.classList.add('wi', 'wi-fahrenheit');
locationSelect.textContent = `Location: ${this.location}`;
tempSelect.textContent = `The temperature is ${Math.round(this.temp)} `;
tempSelect.appendChild(newI);
descSelect.textContent = `Description: ${this.description.charAt(0).toUpperCase()}${this.description.slice(1)}`;
humidSelect.textContent = `Humidity: ${Math.round(this.humidity)}%`;
this.handleEvents();
}
private handleEvents(): void {
let fahBtn = document.querySelector(".fah1");
let temperature = document.querySelector(".temp");
let newI = document.createElement("i");
let celBtn = document.querySelector(".cel2");
fahBtn.addEventListener("click", () => {
temperature.textContent = `The temperature is ${Math.round(this.temp)} `;
newI.classList.remove("wi-celsius");
newI.classList.add("wi", "wi-fahrenheit");
let classes = newI.classList;
temperature.appendChild(newI);
});
celBtn.addEventListener("click", () => {
this.celsius = Math.round((this.temp - 32) * 5 / 9);
temperature.textContent = `The temperature is ${this.celsius} `;
let classes = newI.classList;
newI.classList.remove("wi-fahrenheit");
newI.classList.add("wi", "wi-celsius");
temperature.appendChild(newI);
});
}
}
let weather = new LocalWeather();
weather.getCoords();
Please let me know of any anti-patterns you see. I appreciate any tips!
Here is the github: https://github.com/capozzic1/local_weather_app_v2
1 Answer 1
Here are a few random suggestions:
How come does the code work if the promise is unhandled?! Should be something like new LocalWeather().getCoords().then(result => { }).catch(error => { });
Use const instead of let whenever possible.
IMO, lookup-based logic should be expressed as one. Using switch is inappropriate.
private iconToImage = {
'01d': 'img923/5914/C8hAMP.jpg',
'01n': 'img924/3455/slHReo.jpg',
'02d': 'img922/8471/LF0cGZ.jpg',
'03d': 'img922/8471/LF0cGZ.jpg',
'04d': 'img922/8471/LF0cGZ.jpg',
'02n': 'img923/7048/Ot7l6k.jpg',
'03n': 'img923/7048/Ot7l6k.jpg',
'04n': 'img923/7048/Ot7l6k.jpg',
'09d': 'img921/375/RMia6h.jpg',
'10d': 'img921/375/RMia6h.jpg',
'09n': 'img922/1227/Lf4qc2.jpg',
'10n': 'img922/1227/Lf4qc2.jpg',
'11d': 'img924/2258/h4eNcE.jpg',
'11n': 'img924/2258/h4eNcE.jpg',
'13d': 'img922/3753/cEf7xg.jpg',
'13n': 'img923/1196/G2MDy6.jpg',
'50d': 'img921/8166/H0mO7r.jpg',
'50n': 'img921/8166/H0mO7r.jpg',
};
private handleBackgroundImg(): void {
const weather = document.querySelector(".weather") as HTMLElement;
const toUrl = imagePath => `url("http://imageshack.com/a/${imagePath}")`;
weather.style.backgroundImage = toUrl(this.iconToImage[this.icon]);
}
Since you're using await/async (which are great), I recommend being consistent about using await carefully. Example:
public async getCoords(): Promise<any> {
const coords = await fetch('https://geoip.nekudo.com/api').then((response) => response.json());
this.location = coords.city;
this.lat = coords.location.latitude;
this.long = coords.location.longitude;
const weatherURL = `https://cors-anywhere.herokuapp.com/http://api.openweathermap.org/data/2.5/weather?lat=${this.lat}&lon=${this.long}&units=imperial&APPID=ef73411c829a4563b61b64e76cb72976`;
return await this.getWeatherData(weatherURL);
^^^^^^^^^^^^
// Missing in source...
}
Lat, long, coord should be spelled out. Shortening does is not helpful.
Also, you probably don't even need to have all three fields (location, lat, long). In your particular case, it seems to be more reasonable to just have a field for the fetch() call result:
this.coordinates = await fetch('https://geoip.nekudo.com/api')
.then(response => response.json());
On contrary, the results of document.querySelector(...) are the great candidates for "caching" in the object fields. That's because both DOM access and manipulation are expensive operations.
Constructor is empty, therefore is redundant. Just remove it until it's not needed.
If you have addEventListener() in your code, you should have the corresponding removeEventListener() as well. Otherwise, you get a memory leak and unnecessary CPU cycles.