Shifting history in command-pattern with undo/redo?

Question

I'm having a problem concerning a command pattern with undo/redo function. The simple problem is, when my history is full, I want to remove the least recently used command from the history and add the new one on execute.

I got this code snippet from my professor:

public class CommandHistory implements CommandInterface{

private static final int MAX_COMMANDS = 2;

private Command[] history = new Command[MAX_COMMANDS];


private int current = -1;

@Override
public void execute(Command command) {
    current++;

    if (current == MAX_COMMANDS){                     // if full, then shift
        for (int i = 0; i < MAX_COMMANDS - 1; i++){
            history[i] = history[i+1];
        }

    }
    history[current] = command;
    history[current].execute();
}

In really doubt the if-clause is incorrect, because the current command index remains 2 and only command at index 0 is shifted to 1. But he says this is the way to go. What am I missing?

Answer 1

The loop itself is fine, but two problems:

1. You're quite correct that when current == MAX_COMMANDS is true and you do the loop, current is incorrect and needs adjusting.

2. From a maintenance perspective, current == MAX_COMMANDS is the wrong comparison, it should be current == history.length. (Otherwise, it's easy to change the initialization of history to use something other than MAX_COMMANDS but forget to change every check like current == MAX_COMMANDS.)

I would check current before incrementing it, and only increment it if you're not shifting the contents down:

public void execute(Command command) {

    if (current == history.length - 1){                     // if full, then shift
        for (int i = 0; i < history.length - 1; i++) {
            history[i] = history[i+1];
        }
    } else {
        current++;
    }
    history[current] = command;
    history[current].execute();
}