Memento in java. Mutable state

Question

I am trying to understand the Memento Pattern. For that purpose i am trying to implement the undo function. The problem is that when ever i save the old state of the originator in the queue and run a different function the save state changes to the current state. I really need help to understand what i am doing wrong. How can i make the vector to be immutable.

This is the memento class.

package memento;

public class Memento
{
    private final boolean[] vectorState;

    public Memento(boolean[] vector) {vectorState = vector;}

    boolean[] getMemento() { return vectorState;}

}

The Originator just need to shift a vector of boolean to the left. (TRUE,FALSE,FALSE) shift left returns: (FALSE,FALSE,TRUE). This is the implementation.

package memento;

public class ShilftLeftOriginator
{
    private boolean[] vector;

    public ShilftLeftOriginator(boolean[] vector) {this.vector = vector;}

    public void execute()
    {
        final boolean firstValue = this.vector[0];
        for (int i = 1; i < this.vector.length; i++) {
            this.vector[i - 1] = this.vector[i];
        }
        this.vector[vector.length - 1] = firstValue;
    }


    public Memento saveToMemento() {return new Memento(vector);}

}

And the caretaker:

package memento;

import java.util.Arrays;
import java.util.Deque;
import java.util.LinkedList;


public final class BooleanVector {
    private boolean[] vector;
    private Deque<Memento> mementoList = new LinkedList<>();

    public BooleanVector(boolean[] inputValues) {
        this.vector = inputValues;
    }


    @Override
    public boolean equals(Object obj) 
    {
        if (obj == null) return false;
        if (!(obj instanceof BooleanVector)) return false;
        BooleanVector otherVector = (BooleanVector) obj;
        return Arrays.equals(this.vector, otherVector.vector);
    }

    public void shiftLeft() 
    {
        ShilftLeftOriginator shiftLeft = new ShilftLeftOriginator(vector);
        mementoList.add(shiftLeft.saveToMemento());
        shiftLeft.execute(); // This is my Problem. After execute ist call the value(vector) in mementoList changes             
    }

    public void undo(){ this.vector =  mementoList.pop().getMemento();}

}

And now the test class and the error that i am receiving.

package memento;

public class Main {

    public static void main(String[] args) {
        boolean[] inputValues = { false, true, false };
        BooleanVector vector = new BooleanVector(inputValues);

        vector.shiftLeft();

        boolean[] expectedValues = new boolean[] { true, false, false };
        BooleanVector expectedVector = new BooleanVector(expectedValues);

        if (!vector.equals(expectedVector)) {
            throw new IllegalStateException(vector.toString());
        } else {
            System.out.println("shiftleft working");
        }

        vector.undo();

        expectedValues = new boolean[] { false, true, false };
        expectedVector = new BooleanVector(expectedValues);

        if (!vector.equals(expectedVector)) {
            throw new IllegalStateException(vector.toString());
        } else {
            System.out.println("undo working");
        }

    }

}

The console output:

shiftleft working
Exception in thread "main" java.lang.IllegalStateException: [true, false, false]
    at memento.Main.main(Main.java:26)

score 1 · Accepted Answer · answered Jun 12 '18 at 23:03

The problem is that you're always manipulating the same array. So if you shift left, you also shift left the array you stored in the Memento object, because it's all the same array.

To solve it, make a copy of the array in the constructor of the Memento object:

public Memento(boolean[] vector) {
    vectorState = Arrays.copyOf(vector, vector.length);
}

Aside from that, you seem to have your classes mixed up. BooleanVector and ShilftLeftOriginator are the originator, while Main is the caretaker.

Thank you very much that was exactly what i was looking for. — DerMann, Jun 13 '18 at 09:41

Oleg Cherednik · Answer 2 · 2018-06-13T11:08:17.900

I can give you a little bit more expandable solution:

class Originator: abstract implementatino of the value T holder

public abstract class Originator<T> {

    private T value;
    private final CareTaker<T> careTaker;

    protected Originator(T value, CareTaker<T> careTaker) {
        this.value = value;
        this.careTaker = careTaker;
    }

    public final T getValue() {
        return value;
    }

    protected final void setValue(T value) {
        careTaker.add(this.value);
        this.value = value;
    }

    public final void undo() {
        if (!careTaker.isEmpty())
            value = careTaker.remove();
    }

}

class BooleanVector: concrete implementation that holds boolean[]

public final class BooleanVector extends Originator<boolean[]> {

    public BooleanVector(boolean[] obj) {
        super(obj, new CareTaker<>());
    }

    public void leftShift() {
        if (getValue() == null || getValue().length == 0)
            return;

        boolean[] arr = new boolean[getValue().length];
        boolean tmp = arr[0];
        System.arraycopy(getValue(), 1, arr, 0, getValue().length - 1);
        arr[arr.length - 1] = tmp;
        setValue(arr);
    }

}

class CareTaker: implementation of values change history - memento

public final class CareTaker<T> {

    private final Deque<Item<T>> stack = new LinkedList<>();

    public void add(T value) {
        stack.push(new Item<>(value));
    }

    public T remove() {
        return stack.pop().data;
    }

    public boolean isEmpty() {
        return stack.isEmpty();
    }

    private static final class Item<T> {

        private final T data;

        public Item(T data) {
            this.data = data;
        }
    }
}

Demo:

public static void main(String[] args) {

    BooleanVector originator = new BooleanVector(new boolean[] { false, true, false });
    System.out.println(Arrays.toString(originator.getValue()));
    System.out.println("---");
    originator.leftShift();
    System.out.println(Arrays.toString(originator.getValue()));
    originator.undo();
    System.out.println(Arrays.toString(originator.getValue()));
}

Output:

[false, true, false]
---
[true, false, false]
[false, true, false]

Memento in java. Mutable state

2 Answers2