Skip to content

Comments

Redux Eval #26

Open
martinsalinas0 wants to merge 24 commits intoprojectshft:mainfrom
martinsalinas0:main
Open

Redux Eval #26
martinsalinas0 wants to merge 24 commits intoprojectshft:mainfrom
martinsalinas0:main

Conversation

@martinsalinas0
Copy link

No description provided.

Comment on lines +30 to +33
setQuery("");
} else {
dispatch(fetchForecast(query));
setQuery("");

Choose a reason for hiding this comment

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

DRY
create a function resetSearchBox that calls setQuery. Will help with readability also.

}
};

function average(arr) {

Choose a reason for hiding this comment

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

very generic function name and argument, be specific

Comment on lines +25 to +26
id: forecastData.city.id,
name: forecastData.city.name,

Choose a reason for hiding this comment

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

if city is undefined, this code will crash

state.error = null;
state.forecastData = action.payload;

console.log(action.payload);

Choose a reason for hiding this comment

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

always remove console logs

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