Pattern usage for increment / decrement React (Hooks)

Question

When asked to increment / decrement a particular value I usually create specific functions to handle this functionality. Like this:

  const increaseQuantity = index => {
    const currentItems = [...stock];

    currentItems[index].quantity += 1;
    setStock(currentItems);
  };

  const decreaseQuantity = index => {
    const currentItems = [...stock];

    if (currentItems[index].quantity > 1) {
      currentItems[index].quantity -= 1;
      setStock(currentItems);
    }
  };

I like this approach because it clearly separates each action. Also, it only takes one single argument which keeps things simple.

But I can also create a single function, that does both things (increment or decrement), but it uses an additional parameter action as well as logic to perform the update:

  const manageQuantity = (index, action) => {
    const currentItems = [...stock];

    if (action === "increase") {
      currentItems[index].quantity += 1;
      setStock(currentItems);
    }

    if (currentItems[index].quantity > 1 && action === "decrease") {
      currentItems[index].quantity -= 1;
      setStock(currentItems);
    }
  };

I feel that the second pattern (single function) is more prone to error. Mainly because the the second argument is a (string).

Which is considered best practice and also follows some established (or well known) design pattern? Also, is one pattern more performant than the other?

Full functioning example below:

const availableItems = [
  {
    name: "iPod Nano",
    capacity: "4GB",
    quantity: 3
  },
  {
    name: "iPod Classic",
    capacity: "30GB",
    quantity: 2
  },
  {
    name: "iPod Mini",
    capacity: "4GB",
    quantity: 5
  }
];

function FirstApp() {
  const [stock, setStock] = React.useState(availableItems);

  const increaseQuantity = index => {
    const currentItems = [...stock];
    currentItems[index].quantity += 1;
    setStock(currentItems);
  };

  const decreaseQuantity = index => {
    const currentItems = [...stock];

    if (currentItems[index].quantity > 1) {
      currentItems[index].quantity -= 1;
      setStock(currentItems);
    }
  };

  return (
    <div className="app">
      <h2>FirstApp</h2>
      {JSON.stringify(stock)}
      <hr />

      {stock.map((item, i) => (
        <div key={item.name}>
          {item.name} | <button onClick={() => increaseQuantity(i)}>+</button>
          <button onClick={() => decreaseQuantity(i)}>-</button>
        </div>
      ))}
    </div>
  );
}

function SecondApp () {
  const [stock, setStock] = React.useState(availableItems);

  const increaseQuantity = index => {
    const currentItems = [...stock];
    currentItems[index].quantity += 1;
    setStock(currentItems);
  };

  const decreaseQuantity = index => {
    const currentItems = [...stock];

    if (currentItems[index].quantity > 1) {
      currentItems[index].quantity -= 1;
      setStock(currentItems);
    }
  };

  const manageQuantity = (index, action) => {
    const currentItems = [...stock];
    if (action === "increase") {
      currentItems[index].quantity += 1;
      setStock(currentItems);
    }

    if (currentItems[index].quantity > 1 && action === "decrease") {
      currentItems[index].quantity -= 1;
      setStock(currentItems);
    }
  };

  return (
    <div className="app">
      <h2>SecondApp</h2>
      {JSON.stringify(stock)}
      <hr />

      {stock.map((item, i) => (
        <div key={item.name}>
          {item.name} |{" "}
          <button onClick={() => manageQuantity(i, "increase")}>+</button>
          <button onClick={() => manageQuantity(i, "decrease")}>-</button>
        </div>
      ))}
    </div>
  );
}


function App() {
  return (
    <React.Fragment>
      <FirstApp />
      <SecondApp />
    </React.Fragment>
  )
}

ReactDOM.render(<App/>, document.getElementById('root'));

.app {
  border: 2px solid grey;
  padding: 10px;
  margin: 10px;
}

<script src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>
<div id="root"></div>

Answer 1

Implementation 1

const increaseQuantity = index => {
  const currentItems = [...stock];

  currentItems[index].quantity += 1;
  setStock(currentItems);
};

const decreaseQuantity = index => {
  const currentItems = [...stock];

  if (currentItems[index].quantity > 1) {
    currentItems[index].quantity -= 1;
    setStock(currentItems);
  }
};

Issues

• currentItems[index].quantity += 1; and currentItems[index].quantity -= 1; are state mutations.
• Any computed state updates should use functional state updates and compute the next state from the current state. If for any reason more than a single increaseQuantity or decreaseQuantity is queued within a single render cycle only one gets applied. Similarly, if both a increaseQuantity and decreaseQuantity are enqueued, one would expect the net gain to be zero, but the last one enqueued is the one applied, so the net gain won't be zero.

Solution

• Use a functional update
• Correctly compute next state

Suggestions

const increaseQuantity = index => {
  setStock(stock =>
    stock.map((el, i) =>
      i === index
        ? {
            ...el,
            quantity: el.quantity + 1
          }
        : el
    )
  );
};

const decreaseQuantity = index => {
  setStock(stock =>
    stock.map((el, i) =>
      i === index
        ? {
            ...el,
            quantity: el.quantity - el.quantity > 1 ? 1 : 0,
            // or
            // quantity: Math.max(0, el.quantity - 1),
          }
        : el
    )
  );
};

Implementation 2

const manageQuantity = (index, action) => {
  const currentItems = [...stock];

  if (action === "increase") {
    currentItems[index].quantity += 1;
    setStock(currentItems);
  }

  if (currentItems[index].quantity > 1 && action === "decrease") {
    currentItems[index].quantity -= 1;
    setStock(currentItems);
  }
};

Issues

• Suffers the same state mutation issue previously mentioned.
• More of a design pattern issue, but you've effectively encoded most of a reducer function.

Solution 1

• Apply same fixes as implementation 1
• Reduce code duplication in function

Suggestions

const manageQuantity = (index, action) => {
  setStock(stock =>
    stock.map((el, i) =>
      i === index
        ? {
            ...el,
            quantity:
              el.quantity + action === "increment"
                ? 1
                : el.quantity > 1
                ? -1
                : 0
            // or
            // quantity: Math.max(0, el.quantity + action === "increment" ? 1 : -1)
          }
        : el
    )
  );
};

Solution 2

• Apply same fixes as implementation 1
• Concert to useReducer react hook

Suggestions

Create action types & creator, and a reducer function.

const ACTIONS_TYPE = {
  INCREMENT: 'INCREMENT',
  DECREMENT: 'DECREMENT',
};

const reducer = (state, action) => {
  switch(action.type) {
    case ACTIONS_TYPE.INCREMENT:
      return state.map((el, i) => i === action.index ? {
        ...el,
        quantity: el.quantity + 1,
      } : el);

    case ACTIONS_TYPE.DECREMENT:
      return state.map((el, i) => i === action.index ? {
        ...el,
        quantity: Math.max(0, el.quantity - 1),
      } : el);

    default:
      return state;
  };
};

const increment = index => ({
  type: ACTIONS_TYPE.INCREMENT,
  index,
});

const decrement = index => ({
  type: ACTIONS_TYPE.DECREMENT,
  index,
});

Use in component

const [state, dispatch] = useReducer(reducer, initialState);

const incrementQuantity = () => index => dispatch(increment(index));
const decrementQuantity = () => index => dispatch(decrement(index));

...

...onClick={incrementQuantity(index)}...

Notice:

• Action types are defined as an ENUM so when used in code the likelihood of typos is reduced since the string text isn't used directly.
• Like its redux big brother, it is a bit boiler-plately, but abstracts and isolates the state computation into a reducer pure function. IMO this improves the readability of the code using it.
• It still applies the pattern of using the current state and some input to compute the next state, still always a returning a new state object.

TL;DR

Complexity is similar in both approaches so it really comes down to familiarity. The former is close to the normal component state pattern while the latter resembles a portion of the redux pattern. One isn't necessarily better than the other, though I'd say for simple state the useState and update functions may be easier to grok for juniors joining your team than the useReducer which works well for more complex state shapes.

Lessons Learned

1. Don't mutate state, use shallow copies of state (and sub-state) and update when necessary.
2. Use functional state updates to ensure multiple state updates enqueued within the same render cycle correctly compute next state from previous state.