Saturday, February 20, 2016

Call the Chain of Responsibilty


Design patterns are pretty boring. Experimenting with design patterns? That's where the fun is!

While messing about with spreadsheet formatting objects (e.g. date, text, numeric), it occurred to me that there might be a Dartier (cooler) solution than what I am currently using:
abstract class CellFormatter {
  CellFormatter nextHandler;

  void processRequest(Event e) {
    if (handleRequest(e)) return;
    if (nextHandler == null) return;

    nextHandler.processRequest(e);
  }

  // Subclasses handle requests as needed
  bool _handleRequest(Event e) => false;
  // ...
}
In this handler, I am both implementing and connecting the successors. The nextHandler property points to the next object in the chain. The processRequest() method is responsible for sending requests to that next object when the current object opts against handling the request (e.g. when a number formatter sees text in a cell).

There is nothing wrong with this approach, but I have a nagging dislike for the naming convention that I have here. One method is called processRequest() and the other is _handleRequest(). "Process" and "handle" both have similar meanings so it takes a little extra noodling here to understand what is happening. Since processRequest() is public, that is a good indication that it is the public interface to the chain. Less obvious is that processRequest() is responsible for connecting to the successor whereas the _handleRequest() is meant solely for subclasses to actually handle the request.

The main problem here is the dual nature of processRequest(). To the outside world, it is… processing the request, so it is well-named in that respect. Actually looking at the code, however, it serves the different purpose of linking successors. And naming _handleRequest() as I have only adds to the confusion. This is not horrible, but I can do better.

Perhaps renaming processRequest() as just process() will help. To the outside world, it retains a similar connotation. Inside the code, it seems more distinct from _handleRequest(). Or maybe…

I can rename it as call().

The beautiful thing about a method named call() in Dart is that automatically makes the current class into a function. That means that I can continue sending the request to the chain successor as with nextHandler.call(e) or I can treat nextHandler as a function itself, nextHandler(e):
abstract class CellFormatter {
  CellFormatter nextHandler;

  void call(Event e) {
    if (_handleRequest(e)) return;
    if (nextHandler == null) return;

    nextHandler(e);
  }
  // ...
}
And, since the call() happens to have the correct signature for event listeners, I can supply the first object in the chain directly to the on-change listener:
  var textFormat = new TextFormatter();
  var dateFormat = new DateFormatter(textFormat);
  var numberFormat = new NumberFormatter(dateFormat);

  container.onChange.listen(numberFormat);
That reads much better—and it wasn't horrible to begin with! When the container sees a change event, it number formats the appropriate cell. The class code is similarly clear: when called, it either handles the request or calls the next successor. Wonderful!

I had thought about moving on from last night's "good enough" chain of responsibility. I am certainly glad I opted to linger a bit longer, this was a nice win. I do wonder if this is the gods' way of telling me to linger more often. Or maybe to call() more often. I'm sure one of those is the moral of this story.

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


Day #101

No comments:

Post a Comment