Command Pattern Exercise: Using addFirst() method fixes wrong undo order

Design Patterns
1

Hi

In the Command Pattern, Mosh uses a Deque instead of a List. For adding new objects he uses the add() method, which is available for both the Deque and the List.

I did it the same way in the Command Pattern exercise. As I added several Command objects to my History object one after the other and then tried to undo them in the correct order with an UndoCommand object, I noticed that the order was not correct.

Finally, when I used the addFirst() method to add objects to my History, everything worked as expected.

Code: See gist.

Question: Has anyone else noticed this and/or have I done something wrong?

2

At least currently, Mosh’s solution file implements it like this using push and pop:

package com.codewithmosh.command;

import java.util.ArrayDeque;
import java.util.Deque;

public class History {
    private Deque<UndoableCommand> commands = new ArrayDeque<>();

    public void push(UndoableCommand command) {
        commands.push(command);
    }

    public UndoableCommand pop() {
        return commands.pop();
    }

    public int size() {
        return commands.size();
    }
}

In the video, he did use add which I just assumed added things to the end of the Deque. That should also work. Personally, I would just use the Stack class which has more obvious semantics, but if it works, it works.

AFAICT, that code is buggy. When you call pop() you are popping things off from the end of the Deque. That means you are undoing things in the order that they were done rather than undoing the most recently done thing.

For example, if I did three actions A, B, and C your history would look like this: [C, B, A]. Then calling pop() would produce A, then B, then C (so acting like a FIFO queue). The way undo mechanisms works, we would want to undo C, then undo B, then undo A (acting like a LIFO stack). So your implementation seems to be doing things in the exact opposite order from what they should be doing.

Cheers!