0

I need help in optimizing this code

eatMushroom = (foundMushroom, startTime, steps) => {
  const updatedMushrooms = this.state.mushrooms;
  updatedMushrooms[foundMushroom.key].remaining = false;
  this.setState({
    mushrooms: updatedMushrooms,
    score: this.state.score + 1
  });

  if (this.totalMushrooms === this.state.score) {
    this.props.setTotalTime(startTime, steps);
    this.props.history.push("/score");
  }
};

I this it's taking a toll on performance when replace complete array in state, while I just want to update a single item.

7
  • you are updating ony score? Commented Oct 28, 2017 at 7:20
  • use spread operator Commented Oct 28, 2017 at 7:22
  • Have a look stackoverflow.com/questions/26253351/… Commented Oct 28, 2017 at 7:22
  • Avoid mutating original state array. Use spread operator like this const updatedMushrooms = [...this.state.mushrooms]; Commented Oct 28, 2017 at 7:24
  • Even in this case we are updating the complete array in the state isn't it? Commented Oct 28, 2017 at 7:37

1 Answer 1

2

For a better practice first of all you should avoid mutating state and if you are gonna need values from state while updating state you should consider using functional state updates. This will help on getting the correct values always.

Another thing to consider is that you are using this.state.score right after setting it. setState is async and can occur after you do your if statement. For this you should consider using callbacks.

Below is the modified version of your code with the suggestions above;

this.setState((prevState) => {
  const mushrooms = Object.assign({}, prevState.mushrooms);
  mushrooms[foundMushroom.key].remaining = false;
  return { mushrooms, score: (prevState.score + 1) };
}, () => {
  if (this.totalMushrooms === this.state.score) {
    this.props.setTotalTime(startTime, steps);
    this.props.history.push("/score");
  }
});

I don't know how are you using this.state.mushrooms value but for better performance you can do a little change. If you want to modify only one property then you should move your properties one level up. mushrooms property is unnecessary in my opinion.

Example Data

Rather then using like below

this.state = {
  mushrooms: {
    mushA: {
      remaining: true
    },
    mushB: {
      remaining: false
    },
    mushC: {
      remaining: true
    }
  }
};

You can use like this

this.state = {
  mushA: {
    remaining: true
  },
  mushB: {
    remaining: false
  },
  mushC: {
    remaining: true
  }
};

This way you can update your state like below. One property at a time and I believe this would result in a better performance updates.

this.setState((prevState) => {
  const mushroom = Object.assign({}, prevState[foundMushroom.key], { remaining: false });
  return { [foundMushroom.key]: mushroom, score: (prevState.score + 1) };
}, () => {
  if (this.totalMushrooms === this.state.score) {
    this.props.setTotalTime(startTime, steps);
    this.props.history.push("/score");
  }
});
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.