Skip to content

Request1#234

Open
bdworkman31 wants to merge 7 commits into
projectshft:masterfrom
bdworkman31:master
Open

Request1#234
bdworkman31 wants to merge 7 commits into
projectshft:masterfrom
bdworkman31:master

Conversation

@bdworkman31
Copy link
Copy Markdown

No description provided.

Comment thread main.js
document.body.style.backgroundColor = "white";
}

let currentTemplate = `<div class = "row current-weather mb-4">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be const

Comment thread main.js
}));

//Change Styling Based on Current Weather w/ Switch Statement
switch (weather.condition) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be extracted to a method, no need to redefine it every time you call this function

Comment thread main.js

fetchWeather(city);

document.querySelector("#search-query").value = "";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if requests fails, this is a bad experience

Comment thread main.js
const fetchWeather = (...args) => {
let url;

if (args.length === 1) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before this, need to trim the args.

Comment thread main.js
return mode[0][0];
};

const fetchWeather = (...args) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not good to pass args like this with 2 different types of arguments.
Define both and then check if passed, or wrap in a different method to make it more readable.

Suggested change
const fetchWeather = (...args) => {
const fetchWeather = (city, coordinates) => {

Comment thread main.js
};

//Find the mode of each nested Array
const mode = (array) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear what mode is referring to.

Comment thread main.js
.then((data) => {
addCurrentWeather(data);

const urlForecast = `https://api.openweathermap.org/data/2.5/forecast?lat=${data.coord.lat}&lon=${data.coord.lon}&appid=d408f27efbf2eb54ef2bd39871d4fe8b`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if data changed, your code will crash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants