# Command Pattern: Does my implementation make the point?

Asking for a review of my implementation of a command pattern. In a Editor, words can be entered and the entering of words can be undone.

General purpose Interface

public interface Command {
void execute();
}


The concrete write word command

// Command
public class WriteCommand implements Command {

private Editor editor;
private String word;

public WriteCommand(String w, Editor e) {
this.word = w;
this.editor = e;
}

public String getWord() {
return this.word;
}

public void execute() {
}

}


The undo command

public class UndoCommand implements Command {

private Editor editor;

public UndoCommand(Editor e) {
this.editor = e;
}

@Override
public void execute() {
editor.removeFromWordList();
}
}


The editor (i.e. receiver, i believe):

// Receiver
public class Editor {

private List<String> wordList;

public Editor() {
wordList = new ArrayList<>();
}

}

public void removeFromWordList() {
this.wordList.remove(this.wordList.size()-1);
}

List<String> getWordList() {
return this.wordList;
}

}


The invoker keeping the command queue:

public class Invoker {
private List<Command> undoList;
private List<Command> redoList;

public Invoker() {
undoList = new ArrayList<>();
redoList = new ArrayList<>();

}

public void executeCommand(Command c) {
c.execute();
}
}


The client responsible for creating the commands:

public class Client {
public static void main(String[] args) {

Editor e = new Editor();

Invoker invoker = new Invoker();

invoker.executeCommand(new WriteCommand("Hello", e));
invoker.executeCommand(new WriteCommand("World", e));
System.out.println(e.getWordList());

invoker.executeCommand(new UndoCommand(e));
System.out.println(e.getWordList());

}
}

• Seems to fit the bill. As general remarks: I would not include a reference to the editor in the Command itself - it makes more sense to make it a parameter for the execute method. That's more flexible, and you would have less trouble e.g. serializing the Command instances that way. Of course, you should not put the UndoCommand instances in the undoList. But to me, this seems to be on the right track. – Maarten Bodewes Jan 27 '20 at 1:28
• Same as @MaartenBodewes, passing the "context" as parameter is more flexible than having it as an instance field. Just for the undo, it seems strange to use removeFromWordList your undo maye be used to undo other changes but also to remove another word; your user may want to have a menu to undo "Hello".. (think of your browser's history, you can go back to teh last "n" entries) – gervais.b Jan 27 '20 at 7:45
• execute() should not take any arguments. Thats correct. Some commands may work over editor, some commands may work over other things. If execute accepted the editor than all commands have to. And that is not "more flexible". On other hand the undo logic should be contained within the commands themselves. Check my PoC PHP implementation, it is much more generic, but it should be well explanatory. github.com/slepic/PhpX/blob/master/src/CommandPattern/Undo/… – slepic Jan 27 '20 at 10:31
• The requirement for command pattern is that the command object contains all required state in itself. Adding parameters to the method breaks this requirement and it stops being a command pattern. – TorbenPutkonen Jan 27 '20 at 12:56
• @TorbenPutkonen The editors state is not part of the state of the command itself. There are certainly reasons to choose one over the other, but that's just bad argumentation. Similarly, I would not want every command in the same queue, so mixing commands for various parts of the program is not recommended. I don't want to have the "Save" action be in my Edit -> Undo queue, for one. – Maarten Bodewes Jan 27 '20 at 14:59