While implementing the new box model for Vex, I came to the point, where I needed a representation for the cursor. I started with a very simple class that knew how to draw the cursor at a given offset. That’s easy to understand, one simple responsibility in one class: SRP check!
But then there happend what always happens when you have a simple and elegant solution in place: there was a new requirement: I needed the ability to move the cursor. Just add that damn feature
The first idea could be to add new methods to
Cursor in order to extend
Cursor‘s behaviour. The cursor should move, then let’s add move methods for that.
I know this is too easy and you don’t want to go for this solution. But I also know that there was a time where this would have been the only solution I would have seen. So I will at least mention here why it is not the solution to go for, just for the case.
Moving is a whole new responsibility for the
Cursor class, that has nothing in common with the already existing responsibility of painting. And, there are a lot of different kinds of movement: left/right, up/down, page up/down, one word forward/backward, just to name a few.
Adding just new methods for each distinct movement will clutter the small elegant Cursor class. It will make the class harder to understand. It will lower the inhibition to add further features, which will make the class even harder to understand and in the end we have a Big Ball of Mud, a God Class or however you would call that mess.
There has to be a better solution.
The SRP and “Composition over Inheritance” lead to another approach: put all the movement implementations into a separate class that takes the responsibilty for moving the cursor. This will leave the Cursor class as it is, isolating it against changes that are related to movement, which also makes the OCP happy – at least in this regard. So let’s try it and call this class
CursorMovement for the further discussion.
When starting with the implementation of
CursorMovement, it is clear very soon, that this approach also has some flaws. The different kinds of movement need different information about the current cursor position:
For left/right it is sufficient to know the current offset, because moving left or right is just a matter of decreasing or increasing the offset.
Moving the cursor in vertical direction requires detailed information about the spatial relation of boxes and the current graphical representation of the cursor. Having the implementation of the movement outside of the Cursor class means that this information somehow has to be made available through
Cursor‘s public interface. This leads to a very tight coupling between
Cursor and the
The need for access to the graphical representation revealed another flaw that was already there in the first approach. Information provided by the graphic framework (e.g. font sizes, text length in pixels) is only available during painting (see Double Buffering in SWT for more details), but the cursor movement is triggered by a key stroke – totally independent of the painting. To solve this in a clean way, vertical movement has to be postponed until the next paint event.
Content-related movement (e.g. one word forward/backward) requires information about the textual content of the document, totally independent of the graphical representation. This adds another dependency from
CursorMovement to a new, so far unrelated entity.
All in all there is no cohesion in
CursorMovement and no reuse of code. The class simply piles up unrelated methods and dependencies. We had this already. Together with the unsolved temporal dependency between the graphical information and the paint event, we can come to the conclusion that this approach is also not suitable.
At this point there are a lot of indicators that something is wrong, but it is difficult to see the way out of this. Some impulse from outside might be helpful.
Solutions have to be simple in order to work
Rich Hickey’s talk “Simple Made Easy” gave me this impulse: queues are simple and a great way to do temporal decoupling. This is the missing part in the puzzle. Now the solution is really straight forward:
- We put each single move into its own class, implementing a simple functional interface (
- When triggered by the key strokes, those move instances are added to a queue in
- After adding the move to the queue, also a redraw of the widget is triggered.
- In its paint method,
Cursorfirst applies the queued moves and then does the actual painting.
This use of (some variant of) the command pattern in conjunction with the queue solved most of the problems:
- The access to internal data of the
Cursorclass is restricted to the call of the execution method of the move implementations. No need to provide data through the public interface of
- Each move implementation can have only the dependencies that are needed. Dependencies of a certain kind of move are more explicit.
- There is no single class that piles up unrelated move implementations and dependencies.
- Moves are executed within the paint handler, this resolves the temporal dependency between the move and the availability of information from the graphic framework.
- Adding new moves is just a matter of adding new classes, no changes in existing classes are required. It is even possible to have an extension point which allows other bundles to provide special move implementations for certain document structures (e.g. semantic tables).
It really pays off to know about clean code development. If you listen to your code and you are able to understand the signs, it will always show you the way to a better solution.
If you interested in more details about the implementation, just have a look into the package
org.eclipse.vex.core.internal.cursor in Vex’s git repository.