2

I have a code that loops through all the orders an updates the is_confirmed property to 1. The thing is I have to loop through all the orders find the one that matches the order id and update it.

My question is there more efficient way to do this without looping through all the objects?

export const orders = (state = [], action) => {
  const { type, payload } = action;

  switch (type) {
    case "NEW_ORDER":
      const { new_order } = payload;

      const new_state = state.concat(new_order);

      //console.log(new_state);

      return new_state;

    case "CONFIRM_ORDER":
      const { index } = payload;

      return state.map((order) => {
        if (order.id === index) {
          return { ...order, is_confirmed: 1 };
        } else {
          return state;
        }
      });
  }

  return state;
};
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
json
  • 177
  • 1
  • 9
  • 1
    Does this answer your question? [How to update single value inside specific array item in redux](https://stackoverflow.com/questions/35628774/how-to-update-single-value-inside-specific-array-item-in-redux) – Emile Bergeron Jan 11 '21 at 20:11
  • 1
    I would highly recommend you consider immutable helpers to update your state to ensure the data integrity of your application, which will tie into proper unit testing – AGE Jan 11 '21 at 20:37
  • Everything looks fine in the question, except what looks like a typo inside the `map`, where it should read `return order` in the `else` part. – Emile Bergeron Jan 11 '21 at 20:39

3 Answers3

0

First of all, it would be best if you make your state an object

export const orders = (state = {orders : []},action)

And access your array as state.orders.

Next, never mutate a state variable, make a copy of it first

let ordersCopy= [...state.orders]

Then you can alter this array and set it to state:

ordersCopy.forEach((order) => {
                    if(order.id === index){
                        ordersCopy.splice(index,1,{...order, is_confirmed: 1})
                    } 
 return {...state, orders: ordersCopy}

And in your other case NEW_ORDER:

return {...state, orders: [...state.orders, new_order]}
Sinan Yaman
  • 5,714
  • 2
  • 15
  • 35
  • Making the state an object is irrelevant. Making a copy is unnecessary since OP was already handling the state correctly. `forEach` isn't better than `map` in this case, I'd argue it's even worse. – Emile Bergeron Jan 11 '21 at 20:41
  • I saw your comment about [Reducers Should Own the State Shape](https://redux.js.org/style-guide/style-guide#reducers-should-own-the-state-shape) and this is something relevant that could be explained in your answer that OP could _consider_. While totally optional, it's something to keep in mind. – Emile Bergeron Jan 12 '21 at 15:01
  • Yep, noticed the mistake and removed the comment. Although my answer could be improved, I think it may benefit others searching for the same problem. That is why I am not deleting it. Thanks for the heads up on the answer. Cheers – Sinan Yaman Jan 12 '21 at 15:56
-1

I would just make a copy of the array and find the index of the matched element using findIndex. Then, update it using brackets to access the element:

case "CONFIRM_ORDER":
   const { index } = payload;

   const ordersCopy = [...state]
   const orderIndex = ordersCopy.findIndex(order => order.id === index)
   ordersCopy[orderIndex].is_confirmed = 1

   return ordersCopy
-2

Create a state with React Class or Hook to useState() Please check here- https://reactjs.org/docs/hooks-intro.html