My solution: Remove an Item from an Array

My solution: Remove an Item from an Array
0

#1

What is wrong with my code. It looks to work as asked. But it does not pass the tests.


const immutableReducer = (state = [0,1,2,3,4,5], action) => {
  switch(action.type) {
    case 'REMOVE_ITEM':
      // don't mutate state here or the tests will fail
      if (action.index) {
      const newState = [...state];
      newState.splice(action.index, 1);
      return newState;
      } else {
        return state;
      }
    default:
      return state;
  }
};

const removeItem = (index) => {
  return {
    type: 'REMOVE_ITEM',
    index
  }
}

const store = Redux.createStore(immutableReducer);
console.log(store.getState());
store.dispatch(removeItem(1));
console.log(store.getState());
store.dispatch(removeItem(4));
console.log(store.getState());
store.dispatch(removeItem());
console.log(store.getState());

Your browser information:

User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.25 Safari/537.36.

Link to the challenge:
https://learn.freecodecamp.org/front-end-libraries/redux/remove-an-item-from-an-array


#2

You code fails when store.dispatch(removeItem(0)) is called and then write store.getState(), it should be [1, 2, 3, 4, 5], but your solution returns [0, 1, 2, 3, 4, 5].

What are you trying to accomplish with the following line?

if (action.index) {

If action.index is 0, the above line will evaluate to false, so you end up just returning the initial value of state without removing the first element.


#3

Hi @RandellDawson,

Thanks for your advice.

I used the if conditinal to see if I could except the case in that removeItem() is called without argument… But I understand now that this is not possible since I declared removeItem to ask at lease for the argument index.

Your explanation regarding index 0 explains all my confusion.
:+1:


#4

// A generic easy to follow solution :wink:

const immutableReducer = (state = [0,1,2,3,4,5], action) => {
switch(action.type) {
case ‘REMOVE_ITEM’:
// don’t mutate state here or the tests will fail
return […state].slice(0,action.index).concat([…state].slice(action.index+1,state.length))
//A copy of state array is sliced from index position 0 to one position short of required item, and this array is concatenated with another sliced array starting from one position ahead of item to end of state array
default:
return state;
}
};

const removeItem = (index) => {
return {
type: ‘REMOVE_ITEM’,
index
}
}

const store = Redux.createStore(immutableReducer);


#5

Why is this considered mutating state? The original state value remains unchanged.

const immutableReducer = (state = [0,1,2,3,4,5], action) => {
  switch(action.type) {
    case 'REMOVE_ITEM':
      let a = [...state]
      a.splice(action.index, 1)
      console.log("return value", a)
      console.log("state", state)
      return a
    default:
      return state;
  }
};

const removeItem = (index) => {
  return {
    type: 'REMOVE_ITEM',
    index
  }
}

const store = Redux.createStore(immutableReducer);
store.dispatch(removeItem(2))

//output
return value,0,1,3,4,5
state,0,1,2,3,4,5

Thanks!


#6

In this specific case, the state is not mutated. However, be careful using splice in such cases because e.g. what if the state array was an array of objects… Then the spread operator won’t be successful in making a deep copy of all the objects. Subsequently, using splice will mutate the original state array. The point is, if you can do what you want with slice or any other inherently non-mutating method, use that instead of methods like splice.


#7

Yes, that is a good point. I’m new to functional programming principles and while I like the idea of it, my initial implementations of feel like I’m making the code harder to read!


#8

Yeah, there is a bit of that sometimes… If something goes wrong, it is easier to debug later, but you still pay a price for it(i.e. creating new variables to copy objects).