Search code examples
javascriptarraysreactjsthistime-travel

state history not updating properly in react


I'm building a tower of Hanoi game to get used to react. I have a state property called "disks", which is an array consisting of 3 arrays of length N (N being the total number of disks). I also have defined a state property "history" which is supposed to contain the history of the disks array like this:

  1. intially: history = [disks(Initial config)]
  2. After 1 move: history = [disks(Initial config), disks(after 1 move)]
  3. After 2 moves: history = [disks(Initial config), disks(after 1 move), disks(after 2 move)] etc.

However, after M moves, the history array looks like this:

history = [disks(after M moves), disks(after M moves), ... , disks(after M moves)].

I can't find my mistake. Would appreciate it if anyone had an idea what's going wrong. Here is the relevant code:

constructor(props){
    super(props);
    let disks = [
      [],
      [],
      []
    ];
    //Initially all disks in first tower
    for(let i=0; i<props.numberOfDisks; i++){
      disks[0].push(i);
    }

    this.state = {
      disks : disks,
      selected : null,
      move: 0,
      history: [disks]
    };
  }

  handleClick(i){
    const disks = this.state.disks.slice();
    const history = this.state.history.slice();
    let move = this.state.move;
    let selected = this.state.selected;
    //if user has not previously selected a tower or selects the same tower again
    if(selected===null || i===selected){
      selected = disks[i].length>0 && i!==selected ? i : null;
      this.setState({
        selected : selected
      });
      return;
    }
    //Check if move is legal
    //index is at bottom is 0 and the largest disk has id 0
    if(disks[i].length === 0 || disks[i][disks[i].length-1] < disks[selected][disks[selected].length-1]){
      //perform move
      disks[i].push(disks[selected].pop());
      move++;
      // I guess this is where it goes wrong, but I can't see why
      this.setState({
        history: history.concat([disks]),
        disks: disks,
        move: move
      });
    }
    this.setState({
      selected: null
    });
    console.log(this.state.history);
  }

Please note that the game is otherwise working, meaning the disks array is updating properly etc... It's just the update of the history array that goes wrong somehow. I tried putting disks.slice() into the history.concat as it seemed to me that the history is somehow storing references to the disks array, but that didn't help.


Solution

  • The problem comes from this:

    disks[i].push(disks[selected].pop());
    

    This mutates the disk at index i in place and mutates the selected disk. Because you store theses references in the history and keep adding references of these objects to the history what you are observing is your game stabilising.

    In order to see a little better into what is going on you could try splitting the handleClick method into several parts.

    function getNewState (oldState, selectedIndex) {
      if (oldState.selected === selectedIndex) {
        return oldState;
      }
      if (isLegalMove(oldState, selectedIndex)) {
        return {
          ...oldState,
          selected: selectedIndex, //updated the index
          disks: updateDisk(oldState.disks, selectedIndex),
          move: oldState.move + 1
        };
      }
      return {
        ...oldState,
        selected: null
      };
    }
    

    You see that I introduced several functions for the different parts isLegalMove and updateDisk, these are to separate the concerns and allow testing to be easier.

    Note regarding the use of Array.prototype.slice: as you noticed it only does a shallow copy of a an array, what this means is if you have a nested object and only do a shallow copy of the outer one and then mutate inside it, the original copy will also be mutated. You may want to create a deepCopy instead.