WebAhead10 / CarChecker

0 stars 0 forks source link

Main car models fetch #5

Closed Karyum closed 3 years ago

Karyum commented 3 years ago

There are a couple of issues with this fetch:

1) The following code:

    if (response.status !== 200) {
      console.warn(
        "Looks like there was a problem. Status Code: " + response.status
      );
      return;
    }

In case there is an error from the API then the site is broken, the user would have nothing todo. so first we need to handle these errors from the .catch so instead of return; we would need throw new Error(response.status); (this was taken from this workshop).

speaking of the .catch this takes us to the next problem

2) The following code:

  .catch(function (err) {
    console.error("Fetch Error -", err);
  });

In case the code reaches the .catch then the user will see nothing, he has no indication that an error has happened.

3) the response.json().then(function (data) {, it's cleaner to have the .then one after the other instead of inside each other.

Like this for example is cleaner

 fetch(`https://pokeapi.co/api/v2/pokemon/${name}`)
          .then(response => {
            if (!response.ok) throw new Error(response.status);
            return response.json();
          })
          // if we get a successful response
          .then(pokemonData => {

A reason for using promises is to avoid callback hell, and if you have .then inside a .then then you creating callback hell structure but with promises 😆

HassanHassouna commented 3 years ago

i will review this again and fix it. thanks alot