fac18 / react_game-rosa-pat-

react game
https://react-game-pat-and-rosa.netlify.com/
0 stars 2 forks source link

getData redundancy #15

Open tonylomax opened 4 years ago

tonylomax commented 4 years ago

Although I like the use of error handling throughout I think you're overcomplicating things. You have a separate function for error handling your API call, and then you have additional error handling in getData for any other issues.

It may be worth changing getData to an async/await function. You canawait until your API call has returned something before moving on to the data manipulation. You can still have your catch statement in there but it may remove the need for the checkResponse function.

I'm not hugely confident this is best practice and the way you've done it looks good but may be worth exploring.

oliverjam commented 4 years ago

Fetch requests don't throw errors for non-200 HTTP status codes, so e.g. a 404 response won't trigger your .catch (technically a 404 is a successful response, just not the one you wanted). This is why you need to check res.status === 200 before proceeding.

tonylomax commented 4 years ago

This is cool, didn't actually know that!

oliverjam commented 4 years ago

Ideally checkResponse should throw an error rather than just log it. That way it would trigger your .catch so you could handle the error