Memento patterns doesn't work correctly

Question

I tried to implement some implementation of Memento pattern in Java.

Example is easy - copy video game as example, when user press F5 save gamer state => after pressing F9 recover last saved gamer state.

Here is output from easy run:

Health:          100
Killed Monsters:   0
Health:           90
Killed Monsters:   2
Health:           81
Killed Monsters:   4
Health:           72
Killed Monsters:   6
Health:           64
Killed Monsters:   8
Health:           57
Killed Monsters:  10

But it is wrong result, because last 2 lines should be:

Health:           90
Killed Monsters:   2

I couldn't figure out what is wrong. Code looks ok.

Here is code snippet:

import java.util.Stack;

class GameState {
    private int health;
    private int killedMonsters;

    public GameState(int health, int killedMonsters) {
        this.health = health;
        this.killedMonsters = killedMonsters;
    }

    public double getHealth() {
        return health;
    }

    public int getKilledMonsters() {
        return killedMonsters;
    }

    public void setHealth(int health) {
        this.health = health;
    }

    public void setKilledMonsters(int killedMonsters) {
        this.killedMonsters = killedMonsters;
    }

    @Override
    public String toString() {
        return  String.format("Health: %1$12d\nKilled Monsters: %2$3d", health, killedMonsters);
    }
}

class GameMemento {
    private GameState gameState;

    public GameMemento(GameState gameState) {
        this.gameState = gameState;
    }

    public GameState getGameState() {
        return gameState;
    }
}

class GameOriginator {
    private GameState gameState = new GameState(100, 0);

    public void play() {
        System.out.println(gameState.toString());
        gameState.setHealth((int)(gameState.getHealth() * 0.9));
        gameState.setKilledMonsters(gameState.getKilledMonsters() + 2);
    }

    public GameMemento saveGame() {
        return new GameMemento(gameState);
    }

    public void loadGame(GameMemento memento) {
        gameState = memento.getGameState();
    }
}

class Caretacker {
    private GameOriginator game = new GameOriginator();
    private Stack<GameMemento> quickSaves = new Stack<>();

    public void shutThisDumbAss() {
        game.play();
    }

    public void F5() {
        quickSaves.push(game.saveGame());
    }

    public void F9() {
        game.loadGame(quickSaves.peek());
    }
}

public class MementoDemo {
    public static void main(String[] args) {
        Caretacker caretacker = new Caretacker();
        caretacker.F5();
        caretacker.shutThisDumbAss();
        caretacker.F5();
        caretacker.shutThisDumbAss();
        caretacker.shutThisDumbAss();
        caretacker.shutThisDumbAss();
        caretacker.shutThisDumbAss();
        caretacker.F9();
        caretacker.shutThisDumbAss();
    }
}

Any suggestions?

Answer 1

The problem is that you are reusing the same GameState reference in the GameMemento object. So when you change property of the GameState object, it is also changing the one that were saved in the GameMemento.

To fix this is necessary to save a copy of the GameState object instead its original reference, change your code as the example below:

public GameMemento saveGame() {
     return new GameMemento(new GameState(gameState.getHealth(), gameState.getKilledMonsters()));
}

You can also use the clone method to copy the current instance of the GameState.

Answer 2

Your stack is having the reference to same GameOriginator object. You save the defensive copy in stack for retreiving it later.

public GameMemento saveGame() {
 return new GameMemento(new GameState(gameState.getHealth(), gameState.getKilledMonsters()));
}