Conversation
| .container { | ||
| width: 100%; | ||
| margin: 0 auto; | ||
| } |
There was a problem hiding this comment.
the whole point of using bootstrap is to use to avoid creating the classes yourself.
| @@ -0,0 +1,220 @@ | |||
| // Set current date | |||
| const date = new Date(); | |||
| const options = { weekday: 'long'}; | |||
There was a problem hiding this comment.
options is too generic, dateOptions is a more specific name
|
|
||
| const formattedDates = []; | ||
|
|
||
| for (let i = 0; i < 5; i++) { |
There was a problem hiding this comment.
next time wrap this in a function to give context of the purpose of this code
| var currentWeather = []; | ||
|
|
||
| var weatherForecast = []; |
There was a problem hiding this comment.
started using const, why not let?
| var searchLocation = document.querySelector('#search-query').value; | ||
| var words = searchLocation.split(' '); | ||
| var city = words.join('/'); |
| }; | ||
|
|
||
| var fetchWeather = function(data) { | ||
| const apiKey = '0e79198cac257d0139d13e84e8617759' |
There was a problem hiding this comment.
api key is the same, can be defined once at the top of the file or passed a param
| renderWeather(); | ||
| }; | ||
|
|
||
| var renderWeather = function() { |
| var fetchPlace = function(city) { | ||
| const url = 'http://api.openweathermap.org/geo/1.0/direct?q='; | ||
| const apiKey = '&appid=0e79198cac257d0139d13e84e8617759' | ||
| const fullUrl = url + city + apiKey; | ||
| fetch(fullUrl, { | ||
| method: 'GET', | ||
| dataType: 'json' | ||
| }) | ||
| .then(data => data.json()) | ||
| .then(data => fetchForecast(data)); | ||
| }; |
There was a problem hiding this comment.
You are doing the same 3 times, you could have a generic function that takes api, url and callback for the data and reuse it.
Important DRY principle.
| var fetchPlace = function(city) { | |
| const url = 'http://api.openweathermap.org/geo/1.0/direct?q='; | |
| const apiKey = '&appid=0e79198cac257d0139d13e84e8617759' | |
| const fullUrl = url + city + apiKey; | |
| fetch(fullUrl, { | |
| method: 'GET', | |
| dataType: 'json' | |
| }) | |
| .then(data => data.json()) | |
| .then(data => fetchForecast(data)); | |
| }; | |
| const fetchPlace = (fullUrl, callback) => { | |
| fetch(fullUrl, { | |
| method: 'GET', | |
| dataType: 'json' | |
| }) | |
| .then(data => data.json()) | |
| .then(data => callBack(data)); | |
| }; |
| dayOne.push({ | ||
| conditions: data.list[0].weather[0].main, | ||
| temp: Math.round(data.list[0].main.temp), | ||
| icon: `https://openweathermap.org/img/wn/${data.list[0].weather[0].icon}@2x.png`, | ||
| day: formattedDates[0] | ||
| }) | ||
|
|
||
| dayTwo.push({ | ||
| conditions: data.list[7].weather[0].main, | ||
| temp: Math.round(data.list[7].main.temp), | ||
| icon: `https://openweathermap.org/img/wn/${data.list[7].weather[0].icon}@2x.png`, | ||
| day: formattedDates[1] | ||
| }) | ||
|
|
||
| dayThree.push({ | ||
| conditions: data.list[15].weather[0].main, | ||
| temp: Math.round(data.list[15].main.temp), | ||
| icon: `https://openweathermap.org/img/wn/${data.list[15].weather[0].icon}@2x.png`, | ||
| day: formattedDates[2] | ||
| }) | ||
|
|
||
| dayFour.push({ | ||
| conditions: data.list[23].weather[0].main, | ||
| temp: Math.round(data.list[23].main.temp), | ||
| icon: `https://openweathermap.org/img/wn/${data.list[23].weather[0].icon}@2x.png`, | ||
| day: formattedDates[3] | ||
| }) | ||
|
|
||
| dayFive.push({ | ||
| conditions: data.list[31].weather[0].main, | ||
| temp: Math.round(data.list[31].main.temp), | ||
| icon: `https://openweathermap.org/img/wn/${data.list[31].weather[0].icon}@2x.png`, | ||
| day: formattedDates[4] | ||
| }) |
There was a problem hiding this comment.
DRY, could have used forEach day of the week.
Helper methods are your friends.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach
| renderForecast(); | ||
| }; | ||
|
|
||
| var renderForecast = function() { |
There was a problem hiding this comment.
also this type of function could have been reused. You can create a more 'dumb' implementation that takes querySelector, template as params and makes the action
2 participants
No description provided.