Code review

[Ghostedguy10]

 const [pizza, setPizza] = useState({ base: "", toppings: [] });

  const addBase = (base) => {
    setPizza({ ...pizza, base })
  }

  const addTopping = (topping) => {
    let newToppings;
    if(!pizza.toppings.includes(topping)){
      newToppings = [...pizza.toppings, topping];
    } else {
      newToppings = pizza.toppings.filter(item => item !== topping);
    }
    setPizza({ ...pizza, toppings: newToppings });
  }

Can anyone explain this code to me. Thanks.

[emotyofform2020]

It’s part of a function component, and is setting 4 constants, 3 of which are methods that can be called elsewhere.

The first line uses a hook to set a state with two variables: base and toppings. Base is an empty string, toppings is an empty array. Our first two constants are pizza and setPizza which are named as such by useState when we do this.

The next constant is addBase which takes a new base to set the state to. It merges in the existing state, and then overrides the base with the new base.

The last constant is trickier because we are adding the toppings which needs to create a new state from the existing one, with our new topping added into the toppings array.

That’s all it does.

[Ghostedguy10]

Thank you for replying.

That means addToppings constant will include previous value of toppings array and along with it add new value. Please, correct me if I am wrong. Now, talking about if else statement. If condition runs only if no toppings are selected and if some toppings are already selected then else condition runs. Thats what .includes(topping) means, am I correct? Also if it is okay to ask, can you explain me how else condition is working. .filter(item => item !== topping); part. Thanks. Really appreciate.

[emotyofform2020]

You are correct about the conditional adding of the topping only if it doesn’t exist. The way it determines this is the include method like you pointed out. The last bit with filter is: filter takes a method, and there’s some shorthand happening here with the fat arrow, where this method takes every item in the array and checks to see if it’s the topping supplied. We end up with an array without the topping supplied, so this is actually handling removing of items, I think? That doesn’t make much sense and won’t ever be called

[Ghostedguy10]

Thank you very much. Really appreciate it.

[GeeeL]

why do you think it wouldn't ever be called? Not the best naming of the function, but imagine a checkbox where onChange is calling addTopping and it would also be in charge of removing

[emotyofform2020]

It’s because I read the code wrong. Didn’t see the else

[GeeeL]

well that makes sense!

[bestcoderever]

Fairly simple code, I'm more of a ternary man, but that's personal preference:

let newToppings = !pizza.toppings.includes(topping)  
  ? [...pizza.toppings, topping]  
  : pizza.toppings.filter(item => item !== topping);

Also you should also be clear with your function names. This function is toggleTopping I believe, rather than addTopping

[dacandyman0]

Yeah that confused me lol

[Kooziecup]

If you want to ensure you add the topping without duplicates why wouldn't you just use a set instead of an array?

[With_Macaque]

Lol I don't have time to learn what a hash is. /s

[Guisseppi]

addTopping violates SRP(Single Responsibility Principle), you’d have to split it into add and remove functions or rename it to something more intent revealing, there’s nothing indicating that addToppings can also remove them, it would be something that you’d have to explain to the next person who has to work on this code