The Coding Flow

Code Clean Or Die Tryin'

How the Cursor Learned to Move

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.

Separate responsibilities

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 CursorMovement class.

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:

Conclusion

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.