Subclass Just to Support One Method?

I kinda like Dart mirrors.

As a long time dynamic language programmer, I didn't think that I would, but it can be quite pleasant. Other times, however...

In the code from last night's video, the call() method in the SimpleCommand class is nice:

import 'dart:mirrors';

class SimpleCommand<T> implements Command {
  T receiver;
  Symbol action;
  List args=[];
  SimpleCommand(this.receiver, this.action, [this.args]);
  void call() {
    reflect(receiver).invoke(action, this.args);
  }
}

This reflects on the object, then invokes a method with arguments. What could be easier or cleaner?

What I do not like is what I did with the History class, which holds a list of undoable commands in an application. The guard clause at the beginning of the add() method is too long and obtuse:

class History {
  // ...
  static void add(Function c) {
    if (!reflect(c).type.instanceMembers.containsKey(#undo)) return;

    _h._undoCommands.add(c);
  }
  // ...
}

What that line does is return when the command does not support an undo() method. That is, commands that cannot undo should not be added to this history list. It works, but...

I cannot work directly with the object mirror—there is no way for the object mirror in Dart to reflect on its methods or properties. So instead, I have to get a class mirror from the object mirror's type property. Then, I get a map of the supported methods with instanceMembers. Finally, I can ask if the list of instance members (methods) includes the undo() by asking if the map contains the #undo symbol. But I do not want to know if it contains the key—I want to know if it does not contain the key—so I have to go all the way back to the beginning of the expression to negate it.

I might simplify slightly by breaking parts out into helper methods or switching from a returning, negated guard clause to a curly-brace enclosing expression. Neither of those options really helps cut through the dense, hard-to-read code. I breezed through this implementation last night in the hopes that something better would appear today. Sadly, I cannot find anything in Dart mirrors that can make this any better.

I could switch my undoable command classes to support a different interface—UndoableCommand instead of Command:

abstract class Command implements Function {
  void call();
}

abstract class UndoableCommand implements Command {
  void call();
  void undo();
}

Then commands that support undo, like move-north, can implement this new interface:

class MoveNorthCommand implements UndoableCommand {
  Robot robot;
  MoveNorthCommand(this.robot);
  void call() { robot.move(Direction.NORTH); }
  void undo() { robot.move(Direction.SOUTH); }
}

Making use of that, my guard clause is much clearer:

class History {
  // ...
  static void add(Function c) {
    if (c is! UndoableCommand) return;

    _h._undoCommands.add(c);
  }
  // ...
}

That is much clearer than my mirror based approach. But I hate it.

It seems crazy to declare a class whose sole purpose is to describe that it supports a single method. Why not just ask it if it supports the method?

In the end, it is a good thing I punted this question until today. I finished the video and this certainly would have side-tracked me. I remain unsure what I would do if presented with this in live code. From a code maintainability perspective, I prefer the mirror approach (I'd likely put the obtuse path to the answer into a helper method). From a performance perspective, I would likely favor the subclass-just-to-support-one-method approach, avoiding mirrors. So it depends.

And last night's quick and dirty solution just might end up being my real solution in many instances.


Maybe you can come up with a better approach on DartPad! https://dartpad.dartlang.org/78fca73a983abe26476c


Day #44

‹prev | My Chain | next›