Static Typing Driven Refactoring

‹prev | My Chain | next›

I found yesterday that there was not much that I needed to do in order to ready for the M2 release of Dart—most of the work that I have been doing recently seems to already be bundled in M2. One thing that the dart_analyzer complained about in my code was some insufficient sub-classing in Hipster MVC. This is not a new problem—it is one that I have been ignoring for quite some time.

The problem is how I propagate events through my stack. I use subclasses of the built-in Event class from the dart:html library. I felt that I could kinda/sorta get away with this because dart:html is required for a client-side library. But at the same time, I always felt a little weird about subclassing a browser event class to propagate my MVC stack events. Warnings like the following were a manifestation of the weirdness:

file:/home/chris/repos/hipster-mvc/lib/hipster_model.dart:135:7: Concrete class ModelEvent has unimplemented member(s) 
    # From Event:
        bool bubbles
        bool cancelBubble
        bool cancelBubble
        bool cancelable
        Clipboard clipboardData
        EventTarget currentTarget
        bool defaultPrevented
        int eventPhase
        bool returnValue
        bool returnValue
        EventTarget target
        int timeStamp
        void $dom_initEvent(String, bool, bool)
        void preventDefault()
        void stopImmediatePropagation()
        void stopPropagation()
   134: 
   135: class ModelEvent implements Event {
              ~~~~~~~~~~

I have two choices here. I can either implement those methods with empty definitions or I can stop subclassing Event. I have the nagging suspicion that building my own event class is the way to go, but I opt for the former tonight. The hope is that I can definitively find a reason that subclassing Event for non-DOM events is a bad thing.

It turns out that the simplest thing actually works. I convert each of those method signatures into empty or false definitions:

class ModelEvent implements Event {
  var type, model;
  ModelEvent(this.type, this.model);

  bool bubbles = false;
  bool cancelable = false;
  bool cancelBubble = false;
  Clipboard clipboardData;
  EventTarget currentTarget;
  bool defaultPrevented = false;
  int eventPhase;
  bool returnValue = false;
  EventTarget target;
  int timeStamp;
  void $dom_initEvent(String _a, bool _b, bool _c) {}
  void preventDefault() {}
  void stopImmediatePropagation() {}
  void stopPropagation() {}
}

And I no longer have any warnings—at least not from that class. I deal with most of the other errors in the same manner, but then I come across:

file:/home/chris/repos/hipster-mvc/lib/hipster_model.dart:167:7: Concrete class ModelEventList has unimplemented member(s)
    # From EventListenerList:
        EventListenerList remove(EventListener)
   166: 
   167: class ModelEventList implements EventListenerList {
              ~~~~~~~~~~~~~~

The event listener list is the nexus of events in Dart. It is a list of callbacks that are invoked when a delete happens (or a create happens, or an add, etc.). It probably makes sense to be able to remove callbacks. Someday I will add this functionality, but right now I do not need it. So I simply throw an unsupported error:

class ModelEventList implements EventListenerList {
  var listeners = [];

  add(fn, [bool useCapture=false]) {
    listeners.add(fn);
  }

  EventListenerList remove(EventListener listener, [bool useCapture=false]) {
    throw UnsupportedError;
  }

  bool dispatch(Event event) { /* ... */ }
}

That eliminates all of the warnings from the model class, but the collection class has very similar problems. In fact, it has the exact same problems because the collection supports generating and dispatching events in the same way. Instead of ModelEvent, it is CollectionEvent. There are some collection events that optionally include the model (e.g. when a model is deleted). Aside from those minor differences, the ModelEvent classes are the same as the CollectionEvent classes.

Say, this seems like a good time to introduce a common superclass, which is exactly what I do. In a new hipster_events.dart library file, I declare 3 abstract classes with as much of the common functionality included:

library hipster_events;

import 'dart:html';

abstract class HipsterEvent implements Event {
  // ...
}

abstract class HipsterEvents implements Events {}

abstract class HipsterEventListenerList implements EventListenerList {
  // ...
}

The HipsterEvent class includes all of the missing Event methods that I just added to ModelEvent so that I do not have to add them to both the soon-to-exist ModelEvent and CollectionEvent classes.

I declare these three classes as abstract primarily because I do not want them being used directly. Also, since Event does not have a generative (normal / non-factory) constructor, I would have to define one if I had not declared the class as abstract.

After importing hipster_events.dart into the hipster_collection library, I can then extend the abstract HipsterEvent class:

class CollectionEvent extends HipsterEvent {
  String type;
  HipsterCollection collection;
  HipsterModel model;

  CollectionEvent(this.type, this.collection, {this.model});
}

After doing the same for the other classes (the collection events and the list of listeners for those events), I have eliminated all of the errors from my code—at least as far as dart_analyzer is concerned.

I have to admit that I find an abstract class (HipsterEvent) implementing an interface (Event) a little awkward. How can it truly implement the interface if it still has abstract methods (methods with no bodies)? The only alternative, however, is to extend the interface. That won't work because I would still be left with an interface. Even if I declare method bodies in that interface, the HipsterEvent subclass / implementer does not see those definitions.

One nice side-effect of moving all of the event code into its own library is that neither the hipster_collection nor the hispter_model libraries need to import 'dart:html' any longer. Both libraries are now composed solely of other Hipster MVC classes or the code defined inside their own libraries. Removing an external dependency makes this approach a winner.

As for whether or not I should define my own classes, that question remains outstanding. But if I do, I only need to make that change in a single location now—the hipster_events library—rather than in two locations. But for the time being, I think I will be content to leave well enough alone. And well enough in this case means functional code that still works in the Dart Comics sample application and passes through dart_analyzer.


Day #604