I am looking for a way to convert this classic js function to an immutable one for use in a useState component. It checks if the id of an item is within an array, if it is, it removes it, else it adds it.
Classic function:

   const addOrRemove = (array, item) => {
      array.indexOf(item) === -1 ? array.push(item) : array.splice(array.indexOf(item), 1);
    }

React function:

array.indexOf(item) === -1 ? setArray(array=>[...array, item]) : setArray(array.filter(arrayitem=>arrayitem.id !== item.id)

Few questions: is it correct? i.e. does it achieve to not mutate state? can it be optimized ? is there a way to do it better a whole different way?

Thanks a lot

CodePudding user response：

The converted "React function" has a naming problem, namely the new item supposed to pass in collide with the every iteration of items inside filter function:

filter(item=> item.id !== item.id)

This is bound to return an empty list, since item.id !== item.id will always be false. Unless you named the new item to something else.

And stick the conditional statement in one-liner makes it hard to read, try this:

function handleAddOrRemove(newItem) {
  const newItemExistInArray = array.indexOf(newItem);

  if(newItemExistInArray) { // -1, 0 is evaluated to false, so this is when item exist
    setArray(array.filter(item=> item.id !== newItem.id);
    return;
  }

  setArray(array => [...array, newItem]);
}

CodePudding user response：

In your existing function, you are using two methods which mutate the array in-place:

const addOrRemove = (array, item) => {
  array.indexOf(item) === -1
    ? array.push(item)
    //      ^^^^^^^^^^
    //      mutates array
    : array.splice(array.indexOf(item), 1);
    //      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    //      mutates array

  // Nothing (undefined) is returned
};

This prevents the function from being pure, and is against one of the primary rules of state in React: Do not modify state directly.

Instead, you can create a new array which is a shallow copy of the existing array, then mutate the copy and return it:

const addOrRemove = (array, item) => {
  const copy = [...array];

  copy.indexOf(item) === -1
    ? copy.push(item)
    : copy.splice(array.indexOf(item), 1);

  return copy; /*
  ^^^^^^^^^^^
  returns a new array */
};

Then, you can use it to update your state (without mutating it) like this:

This example uses the functional update form of the argument provided to the setter function returned by the useState hook:

setArray(array => addOrRemove(array, item));