How can I have a cleaner filter with multiple if else inside a useEffect-CodePudding

I've got a filter function inside a useEffect hook that changes the content displayed according to the state set by one of the three dropdowns managed by this hook.
It currently works but I'm in the process of refactoring my code and I'm sure that there is a cleaner and more efficient way to do what I want to do.
Here's the code :

useEffect(() => {
    setFilteredPokedex(
      pokedex
        .filter((pokedex) => {
          return (
            type === `all` ||
            pokedex.types.map((pt) => pt.type.name).includes(type)
          );
        })
        .filter((pokedex) => {
          if (form === `default` && generation === `all`) {
            setOffset(0);
            setLimit(905);
            return pokedex;
          } else if (form === `regional - alola`) {
            setOffset(995);
            setLimit(30);
            return pokedex?.name?.includes(`alola`);
          } else if (form === `regional - galar`) {
            setOffset(1065);
            setLimit(25);
            return pokedex?.name?.includes(`galar`);
          } else if (form === `regional - hisui`) {
            setOffset(1133);
            setLimit(20);
            return (
              pokedex?.name?.includes(`hisui`) ||
              pokedex?.name?.includes(`origin`)
            );
          } else if (form === `mega`) {
            setOffset(937);
            setLimit(70);
            return (
              pokedex?.name?.includes(`-mega`) ||
              pokedex?.name?.includes(`primal`)
            );
          } else if (form === `gmax`) {
            setOffset(1099);
            setLimit(35);
            return pokedex?.name?.includes(`gmax`);
          } else if (generation === `gen1`) {
            setOffset(0);
            setLimit(151);
            return pokedex;
          } else if (generation === `gen2`) {
            setOffset(151);
            setLimit(100);
            return pokedex;
          } else if (generation === `gen3`) {
            setOffset(251);
            setLimit(135);
            return pokedex;
          } else if (generation === `gen4`) {
            setOffset(386);
            setLimit(107);
            return pokedex;
          } else if (generation === `gen5`) {
            setOffset(493);
            setLimit(156);
            return pokedex;
          } else if (generation === `gen6`) {
            setOffset(649);
            setLimit(72);
            return pokedex;
          } else if (generation === `gen7`) {
            setOffset(721);
            setLimit(88);
            return pokedex;
          } else if (generation === `gen8`) {
            setOffset(809);
            setLimit(96);
            return pokedex;
          }
        }),
    );
  }, [
    pokedex,
    form,
    generation,
    type
  ]);

I don't really know what I should do to make it better because in my mind it's the best way to do it (I've wrote the code so I understand it but I know that someone new to this code would be confused by it). I've thought of using a switch statement but Idk if it's gonna be better.
I've also thought about replacing the "setOffset" and "setLimit" with "return pokedex.id < x && pokedex.id > y".

Edit : some code missing Dropdowns :

<PokedexDropdown>
          <label htmlFor="form">Form</label>
          <select
            name="form"
            id="form"
            value={form}
            onChange={(e) => {
              setForm(e.target.value);
              setGeneration(`all`);
              setType(`all`);
            }}
          >
            <option value="default">Default</option>
            <option value="regional - alola">Regional - Alola</option>
            <option value="regional - galar">Regional - Galar</option>
            <option value="regional - hisui">Regional - Hisui</option>
            <option value="mega">Mega</option>
            <option value="gmax">Gmax</option>
          </select>
        </PokedexDropdown>

        <PokedexDropdown className={form === `default` ? `` : `hidden`}>
          <label htmlFor="generation">Generation</label>
          <select
            name="generation"
            id="generation"
            value={generation}
            onChange={(e) => {
              setGeneration(e.target.value);
              setForm(`default`);
              setType(`all`);
            }}
          >
            <option value="all">All</option>
            <option value="gen1">Generation I</option>
            <option value="gen2">Generation II</option>
            <option value="gen3">Generation III</option>
            <option value="gen4">Generation IV</option>
            <option value="gen5">Generation V</option>
            <option value="gen6">Generation VI</option>
            <option value="gen7">Generation VII</option>
            <option value="gen8">Generation VIII</option>
          </select>
        </PokedexDropdown>

        <PokedexDropdown>
          <label htmlFor="type">Type</label>
          <select
            name="type"
            id="type"
            value={type}
            onChange={(e) => {
              setType(e.target.value);
            }}
          >
            <option value="all">All</option>
            <option value="bug">Bug</option>
            <option value="dark">Dark</option>
            <option value="dragon">Dragon</option>
            <option value="electric">Electric</option>
            <option value="fairy">Fairy</option>
            <option value="fighting">Fighting</option>
            <option value="fire">Fire</option>
            <option value="flying">Flying</option>
            <option value="ghost">Ghost</option>
            <option value="grass">Grass</option>
            <option value="ground">Ground</option>
            <option value="ice">Ice</option>
            <option value="normal">Normal</option>
            <option value="poison">Poison</option>
            <option value="psychic">Psychic</option>
            <option value="rock">Rock</option>
            <option value="steel">Steel</option>
            <option value="water">Water</option>
          </select>
        </PokedexDropdown>

What is returned by the pokedex array : pokedex

CodePudding user response：

I will try to provide you some ideas on how to make the code "cleaner" as you requested in your post.

Please have in consideration that I can test small pieces to see if works since I don't have access to the whole code.

I noticed that you have an offset and limit values, based on which generation and form you have. So, lets see if we can make this smaller.

// First, let store those values in an easy accessible array of objects.
const gen = [
  { generation: "gen1", offset: 0, limit: 151 },
  { generation: "gen2", offset: 151, limit: 100 },
  { generation: "gen3", offset: 251, limit: 135 },
  { generation: "gen4", offset: 386, limit: 107 },
  { generation: "gen5", offset: 493, limit: 156 },
  { generation: "gen6", offset: 649, limit: 72 },
  { generation: "gen7", offset: 721, limit: 88 },
  { generation: "gen8", offset: 809, limit: 96 }
];

// TEST & EXAMPLE
let inputs = ["wrong", "gen1", "gen2", "gen3", "gen4", "gen5", "gen6", "gen7", "gen8"];
for (const input of inputs){
    console.log("Input: " , input);
    let result = gen.filter((elem) => elem.generation == input)
    console.log("Offset: ", result.offset, ", Limit: ", result.limit);
}

As you might noticed when running the code, we can reduce those if-statements used for the generations.

Next, we see how to take care of the rest that have form. We can still apply something similar to above:

// First, let store those values in an easy accessible array of objects.
const gens = [
  { generation: "gen1", form: null, offset: 0, limit: 151 },
  { generation: "gen2", form: null, offset: 151, limit: 100 },
  { generation: "gen3", form: null, offset: 251, limit: 135 },
  { generation: "gen4", form: null, offset: 386, limit: 107 },
  { generation: "gen5", form: null, offset: 493, limit: 156 },
  { generation: "gen6", form: null, offset: 649, limit: 72 },
  { generation: "gen7", form: null, offset: 721, limit: 88 },
  { generation: "gen8", form: null, offset: 809, limit: 96 },
  { generation: "all", form: "default", offset: 0, limit: 905 },
  { generation: null, form: "regional - alola", offset: 995, limit: 30 },
  { generation: null, form: "regional - galar", offset: 1065, limit: 25 },
  { generation: null, form: "regional - hisui", offset: 1133, limit: 20 },
  { generation: null, form: "mega", offset: 937, limit: 70 }, 
  { generation: null, form: "mega", offset: 937, limit: 70 },  
];


// Lets get us some testing inputs by copying the content into an new object to modify
let inputs = JSON.parse(JSON.stringify(gens));
inputs = inputs.map(item => {
  delete item.offset;
  delete item.limit;
  return item
});

// Add some extra examples
inputs.push({ generation: null, form: null});
inputs.push({ generation: "apples", form: "banana"});

// Print the input and the result output to verify if works.
for (const input of inputs){
  console.log("Input: ", input);
  let result = gens.filter((e) => {
    return e.generation == input.generation && e.form == input.form    
  })
  console.log("Result:", result);  
}

Now, let modify this code:

.filter((pokedex) => {
          // Special cases first
          if (form === `regional - alola`) {
            setOffset(995);
            setLimit(30);
            return pokedex?.name?.includes(`alola`);
          } else if (form === `regional - galar`) {
            setOffset(1065);
            setLimit(25);
            return pokedex?.name?.includes(`galar`);
          } else if (form === `regional - hisui`) {
            setOffset(1133);
            setLimit(20);
            return (
              pokedex?.name?.includes(`hisui`) ||
              pokedex?.name?.includes(`origin`)
            );
          } else if (form === `mega`) {
            setOffset(937);
            setLimit(70);
            return (
              pokedex?.name?.includes(`-mega`) ||
              pokedex?.name?.includes(`primal`)
            );
          } else if (form === `gmax`) {
            setOffset(1099);
            setLimit(35);
            return pokedex?.name?.includes(`gmax`);
          } 
          return regularCases(pokedex, form, generation);
        }),

Let's assume that the function regularCases(pokedex, form, generation) holds the code I provide you above as an example.

Now, lets make a separate function to take care of the special cases:

.filter((pokedex) => {
  let rtn = specialCases(pokedex, form, generantion);
  return (rtn) ? rtn : regularCases(pokedex, form, generation);
}),

The reason I did this is to separate concerns.

Now, you can write a separate function that take care of these special cases.

I hope these ideas helps you to make your code "cleaner".

CodePudding user response：

for the generation part, I think it's better to store the offset and limit numbers in a constants.js file.

constants.js:

export const genToLimitOffsetMap = {
gen1:{
    offset: 0,
    limit: 151
    } 
... // other gens
}

and replace this

else if (generation === `gen1`) {
            setOffset(0);
            setLimit(151);
            return pokedex;
          } else if (generation === `gen2`) {
            setOffset(151);
            setLimit(100);
            return pokedex;
          } else if (generation === `gen3`) {
            setOffset(251);
            setLimit(135);
            return pokedex;
          } else if (generation === `gen4`) {
            setOffset(386);
            setLimit(107);
            return pokedex;
          } else if (generation === `gen5`) {
            setOffset(493);
            setLimit(156);
            return pokedex;
          } else if (generation === `gen6`) {
            setOffset(649);
            setLimit(72);
            return pokedex;
          } else if (generation === `gen7`) {
            setOffset(721);
            setLimit(88);
            return pokedex;
          } else if (generation === `gen8`) {
            setOffset(809);
            setLimit(96);
            return pokedex;
          }

with

else{
setOffset(genToLimitOffsetMap[generation].offset)
setLimit(genToLimitOffsetMap[generation].limit)
}

and same approache may be effective for the rest of the code

CodePudding user response：

So for gen filter you can just check if the filter starts with "gen" and extract the number after and display based on that for example if each gen has exactly 150 items you can multiply gen with 150 to get the offset, for other filters you can make input a variable and pass it into the pokedex?.name?.includes(variable).

If generations don't have the matching number of items you can store the number of items in a hashmap and extract it from there with int extracted from genX argument.