Friday, December 11, 2015

Multiple Command Undos


Do multiple receivers complicate life in the command pattern? My suspicion is that they do, but let's see.

I am still working with last night's VelvetFogMachine and Playlist receiver classes in Dart. Menu items from VelvetFogMachine's user inteface initiate commands that use these two receivers in order to aid in the assembly of the ultimate collection of Mel Tormé songs. Because everyone loves Mel.

From last night, the menu items supply arguments to the commands:
// ...
  menu.call(play, ['It Had to Be You']);
  // ...
  menu.call(addToPlaylist, ['Blue Moon']);
  // ...
My initial inclination here is that the history mechanism, currently residing in the Menu class, is going to need to remember these arguments:
class Menu {
  List _history = [];

  void call(Command c, [List args]) {
    if (args != null)
      c.call(args);
    else
      c.call();

    _history.add([c, args]);
  }
  // ...
}
But that is premature feature creep. For now, I stick with just adding the command to the undo history list:
    _history.add(c);
An undo can pop the last command off the list and invoke its undo():
class Menu {
  // ...
  void undo() {
    var h = _history.removeLast();
    print("Undoing $h");
    h.undo();
  }
  // ...
}
I will start by undoing the playing of the current song:
  menu.call(play, ['It Had to Be You']);
  menu.call(play, ['Cheek to Cheek']);
  menu.undo();
In this scenario, the velvet fog machine should stop playing Cheek to Cheek (something no self-respecting Mel fan would ever do) and revert to playing the previous song, It Had to Be You. I had thought the history would have to remember the song arguments, but I can now that I was wrong.

When the PlayCommand is run, it can be responsible for knowing the previous song, which it can get from the VelvetFogMachine:
class PlayCommand implements Command {
  VelvetFogMachine machine;
  String _prevSong;

  PlayCommand(this.machine);

  void call([List args]) {
    _prevSong = machine.currentSong;
    machine.play(args.first);
  }
  // ...
}
The undo() method can then tell the velvet fog machine to play that previous song:
class PlayCommand implements Command {
  // ...
  void undo() {
    machine.play(_prevSong);
  }
}
That does the trick. When I run the following commands:
  menu.call(play, ['It Had to Be You']);
  menu.call(play, ['Cheek to Cheek']);
  menu.undo();
The resulting output is:
Play It Had to Be You
Play Cheek to Cheek
Undoing Instance of 'PlayCommand'
Play It Had to Be You
Despite that success, I am still unconvinced that the history can do without the command arguments. Let's see what happens when I try to remove and item from the playlist:
  var playlist = new Playlist([
    '\'Round Midnight',
    'It Don\'t Mean A Thing (If It Ain\'t Got That Swing)',
    'New York, New York',
    'The Lady is a Tramp'
  ]);
  menu.call(removeFromPlaylist, ['New York, New York']);
  menu.undo();
  menu.call(play, [playlist]);
This turns out to be tricky, but not for any reason associated with the amount of information stored in history. Rather, the problem is one of reference vs. value variable assignment.

The problem arises when storing the previous playlist when the remove command is invoked:
class PlaylistRemoveCommand implements Command {
  Playlist playlist, _prev;

  PlaylistRemoveCommand(this.playlist);

  void call([List args]) {
    _prev = playlist;
    print('==> [remove] $args');
    playlist.remove(args.first);
  }
}
Since Dart passes arguments by reference, both _prev and playlist refer to the same object. So if the undo command tries to undo:
class PlaylistRemoveCommand implements Command {
  // ...
  void undo() {
    playlist.songs = _prev.songs;
  }
}
I am out of luck—_prev.songs is missing New York, New York.

To work around this, I have to add a clone() method to the Playlist that copies the values in the playlist:
class Playlist {
  List<String> songs = [];
  // ...
  Playlist clone() => new Playlist(songs.map((s)=> s).toList());
}
Updating the command's call() method to clone the playlist before storing it as a representation of the previous state should do the trick:
class PlaylistRemoveCommand implements Command {
  Playlist playlist, _prev;

  PlaylistRemoveCommand(this.playlist);

  void call([List args]) {
    _prev = playlist.clone();
    print('==> [remove] $args');
    playlist.remove(args.first);
  }

  void undo() {
    playlist.songs = _prev.songs;
  }
}
Now, undoing the remove:
  menu.call(removeFromPlaylist, ['New York, New York']);
  menu.undo();
Results in:
==> [remove] [New York, New York]
Undoing Instance of 'PlaylistRemoveCommand'
Play 'Round Midnight
     It Don't Mean A Thing (If It Ain't Got That Swing)
     New York, New York
     The Lady is a Tramp
So, at least for the examples that I have so far, the command objects are able to get enough information from their receivers when assembling undo information. The need for cloning receivers was not unexpected—the Gang of Four book specifically mentions this as being needed. So the answer to my original question seems to be that multiple receiver type really has no effect on the complexity of command pattern code.

I stop here for the night. Up tomorrow: I think that I have some naming issues with my menu items. I will take a closer look. Until then...

Play with the code on DartPad: https://dartpad.dartlang.org/79cd403814429ab2a2cf.


Day #30


No comments:

Post a Comment