Weather app eval submission#210
Conversation
| const data = await fetchData(url); | ||
|
|
||
| // If location is not found | ||
| if(data.length === 0) return -1; |
There was a problem hiding this comment.
its better thrown an error and catch it in later.
| }; | ||
|
|
||
| const getCurrentWeatherData = async function (lon, lat, unit="imperial") { | ||
| const returnData = {} |
|
|
||
| const getCurrentWeatherData = async function (lon, lat, unit="imperial") { | ||
| const returnData = {} | ||
| const url = `https://api.openweathermap.org/data/2.5/weather?lat=${lat}&lon=${lon}&units=${unit}&appid=${APIKEY}` |
| const url = `https://api.openweathermap.org/data/2.5/weather?lat=${lat}&lon=${lon}&units=${unit}&appid=${APIKEY}` | ||
| const data = await fetchData(url); | ||
| returnData.name = data.name; | ||
| returnData.temp = data.main.temp; |
There was a problem hiding this comment.
if by any reason, main is not being returned, this will crash. Safer to use ?
| returnData.temp = data.main.temp; | |
| returnData.temp = data.main?.temp; |
| const data = await fetchData(url); | ||
| returnData.name = data.name; | ||
| returnData.temp = data.main.temp; | ||
| const {description, icon} = data.weather[0]; |
There was a problem hiding this comment.
| const {description, icon} = data.weather[0]; | |
| const { description, icon } = data.weather[0]; |
check your spacing
| const getCurrentWeatherData = async function (lon, lat, unit="imperial") { | ||
| const returnData = {} | ||
| const url = `https://api.openweathermap.org/data/2.5/weather?lat=${lat}&lon=${lon}&units=${unit}&appid=${APIKEY}` | ||
| const data = await fetchData(url); |
There was a problem hiding this comment.
Data and return data are very generic names, you want to use something more explicit.
| returnData.temp = data.main.temp; | ||
| const {description, icon} = data.weather[0]; | ||
| const output = {...returnData, weather: description, iconCode: icon}; | ||
| return output; |
There was a problem hiding this comment.
same as you wont call your inputs - 'input', don't call it output. Can be:
currentWeatherData same as your function.
makes sense?
| dateArr.push(itm); | ||
| dateSeparatedArr.push(dateArr); | ||
| dateArr = []; |
There was a problem hiding this comment.
this is confusing and prone to bugs. Will be hard to maintain.
A lot is being mutated here.
| console.log("setDefaultBtn clicked"); | ||
| console.log(weatherComponent.name); |
| console.log(weatherComponent.name); | ||
| localStorage.setItem("WEATHERPROJECT", JSON.stringify({"name": weatherComponent.name})); | ||
| $("#default-city-text").text(`Default city: ${weatherComponent.name}`); | ||
| toggleModal("Success!", "New Default location has been set!"); |
There was a problem hiding this comment.
you can set incorrect city names as default. Should validate!
There was a problem hiding this comment.
Basically, I think you need to be able to set as default ONLY if you have that city already populated with data, otherwise that button stays disabled
2 participants
No description provided.