Saturday, August 17, 2013

Intentionally Sloppy Outside-In Test Driven Development in Dart


With the last (fingers crossed) copy edits for 3D Game Programming for Kids out of the way, it is nearly time for the final push to get Dart for Hipsters updated for the most recent Dart.

Well, that's not entirely accurate. I still have to make a book trailer or two for 3D Kids as well as other promotional material. But still, I need to redouble my efforts to get Dart for Hipsters updated. The recent exploration of events in Dart—keyboard events in particular—has been illuminating, but will be of little help towards that goal. Since Dart has switched over to streams, it seems my best course of action is to do the same with what used to be the events chapter.

But I cannot just leave my recent efforts hanging. I need to apply some of what I have learned to the ctrl_alt_foo keyboard shortcut package for Dart. And what I have primarily learned is that I no longer have any desire to be mucking with on-key-up, on-key-down, or on-key-press. They are no better in Dart than they are in JavaScript. It should be noted that this will not always be the case, the KeyEvent class will eventually normalize at least a portion of this, but it is not ready yet.

For now, I will shift the focus of ctrl_alt_foo from trying to fix KeyEvent and KeyboardEventStream (which produces KeyEvents). Experiments over the last two days have shown me that KeyDown is the only thing that has some consistency and can be used for keyboard shortcuts, so I will make ctrl_alt_foo a higher level wrapper for keydown streams.

By way of example, I want to replace this:
    _keyDownSubscription = KeyboardEventStreamX.onKeyDown(document).listen((e) {
      if (e.isEscape) {
        _hideMenu();
        _hideDialog();
      }

      if (e.isCtrl('N') || e.isCommand('N')) {
        new NewProjectDialog(this).open();
        e.preventDefault();
      }
      if (e.isCtrl('O') || e.isCommand('O')) {
        new OpenDialog(this).open();
        e.preventDefault();
      }
      if (e.isCtrlShift('H') || e.isCommandShift('H')) {
        toggleCode();
        e.preventDefault();
      }
    });
With something along the lines of:
    Keys.map({
      'Esc':       (){ _hideMenu(); _hideDialog(); },
      'Ctrl+N':       ()=> new NewProjectDialog(this).open(),
      'Ctrl+O, ⌘+O':  ()=> new OpenDialog(this).open(),
      'Ctrl+Shift+H': ()=> toggleCode()
    });
That is a lot simpler and easier to read. Somewhat lost in the complexity above is the need to prevent default handling of events. So ctrl_alt_foo will have to preventDefault() event actions. Also, there is no stream subscription returned by this approach. I am using that to remove keyboard listeners withing the calling context. In this new, higher-level approach, I am going to add support for Keys.cancel() which will take care of removing and keyboard shortcut event listeners.

I am going to use outside-in testing for this. I start with the highest level interface that I hope to support, even though much work will need to be done underneath it before that high level interface will work. That high-level test sets up the keyboard mapping as above and attempts to type ⌘-O:
  group("Keys", (){
    test("can establish shortcut listerner with a simple map", (){
      Keys.map({
        'Esc':          (){},
        'Ctrl+N':       (){},
        'Ctrl+O, ⌘+O':  expectAsync0((){}),
        'Ctrl+Shift+H': (){}
      });

      typeCommand('O');
    });
  });
That test fails with:
ERROR: Keys can establish shortcut listerner with a simple map
  Test failed: Caught No top-level getter or setter 'Keys' declared.
  
  NoSuchMethodError : method not found: 'Keys'
Even if I declare a simplified version of the class:
class Keys {
  static void map(Map shortcuts) {
    // code here...
  }
}
The test still fails because I need that map() static method to create a bunch of listeners. But first, I need something underneath this code—something to listen for the specified shortcut and to then invoke the supplied callback. For that, I leave my current test failing and move down. I write a test for how I would like a new Shortcut class to work:
    test("can establish a keyboard shortcut", (){
      new ShortCut('A', expectAsync0((){}), isCtrl: true);
      typeCtrl('A');
    });
In this test, I create a new shortcut for the key “A” and require that the Control key be held down. For the callback, I supply Dart's expectAsync0() method, meaning that the callback should be invoked with zero arguments. I am not going to supply the keyboard event itself to the callback since this will only fire when all of the specifics of the event are already known.

To make that test pass, I write a small Shortcut class:
class ShortCut {
  String char;
  bool isCtrl, isShift, isMeta;
  StreamSubscription subscription;
  var cb;

  ShortCut(
    this.char,
    this.cb,
    {
      this.isCtrl: false,
      this.isShift: false,
      this.isMeta: false
    })
  { this._createStream(); }

  void _createStream() {
    KeyboardEventStreamX.onKeyDown(document).listen((e) {
      if (!e.isKey(char)) return;

      if (e.isCtrl  != isCtrl) return;
      if (e.isShift != isShift) return;
      if (e.isMeta  != isMeta) return;

      cb();
    });
  }
}
The Shortcut() constructor uses my preferred long constructor formatting and then creates the stream to listen for events. The _createStream() private method uses the KeyboardEventStreamX class that I have been building. Originally intended as a top-level class to be exported from ctrl_alt_foo, it will now play a supporting role. The listener listens for all keyboard events that reach a page's document element. If the character, control, shift and meta modifiers match the event, then the callback is invoked.

With that, I have my “inside” test passing:
PASS: ShortCut can establish a keyboard shortcut
Now I can work my way back out to the still failing test and hope to make it pass.

I made my test intentionally difficult, opting to get the second part of a multi-key shortcut working. On top of that, I am trying to get a Mac-style keyboard shortcut working:
    test("can establish shortcut listerner with a simple map", (){
      Keys.map({
        'Esc':          (){},
        'Ctrl+N':       (){},
        'Ctrl+O, ⌘+O':  expectAsync0((){}),
        'Ctrl+Shift+H': (){}
      });

      typeCommand('O');
    });
That would be risky behavior on my part if I were driving toward a completed implementation, but the connection between the Keys and ShortCut classes is a little fuzzy in my mind. So I use this test to get the code working as quickly as possible and will refactor into a stable API another day.

To get this “outside” test to pass, I create a Keys class that looks like:
class Keys {
  static void map(Map shortcuts) {
    shortcuts.forEach(split);
  }

  static split(key, callback) {
    var keys = key.split(new RegExp(r'\s*,\s*'));
    keys.forEach((k) { toShortCut(k, callback); });
  }

  static toShortCut(String k, callback) {
    var parts = k.
      replaceAll('⌘', 'Meta').
      replaceAll('Command', 'Meta').
      split('+');

    var key = parts.removeLast();

    parts.sort();
    switch (parts.join('+')) {
      case '':
        new ShortCut(key, callback);
        break;
      case 'Ctrl':
        new ShortCut(key, callback, isCtrl: true);
        break;
      case 'Meta':
        new ShortCut(key, callback, isMeta: true);
        break;
      case 'Shift':
        new ShortCut(key, callback, isShift: true);
        break;
      case 'Ctrl+Shift':
        new ShortCut(key, callback, isCtrl: true, isShift: true);
        break;
      case 'Meta+Shift':
        new ShortCut(key, callback, isCtrl: true, isShift: true);
        break;
      default:
        throw 'Unsupported key combo';
    }
  }
}
That does work—my tests all pass in ctrl_alt_foo and even in the ICE code editor when this API is linked in (I love being able to link an active API into a separate codebase in Dart). But I am not sure that I have the API quite right.

It is not horrible, but anytime that I find myself writing a number of static methods, I am fairly certain that another object is trying to escape. In this case, it may be as simple as needing to move the toShortCut() static method into the ShortCut class—possibly as a named constructor (new ShortCut.fromString(...)?).

With all of my tests passing and my original questions satisfactorily answered, I will call it a night here. Before deciding on refactoring tomorrow, I think it wise to explore error handling better than I have done so here. Perhaps that exploration will give me a better idea of how I want this to work.


Day #846

No comments:

Post a Comment