Say What You Mean September 19th, 2015
Patrick Stein

There is a scene in the movie The Birdcage where the son tells his father that he (the son) has met a girl and is going to get married. The father begins gulping down the glass of wine that he has in hand. The son asks, Are you upset? The father finishes the glass of wine and says, Let me tell you why.

Here is a function that I wrote several years ago.

(sheeple:defreply mouse-move ((item =draggable=) xx yy)
  (let ((dragging (dragging item)))
    (when dragging
      (let ((dx (- xx (car dragging)))
            (dy (- yy (cdr dragging))))
        (incf (offset-x item) dx)
        (incf (offset-y item) dy))
      (let ((pp (parent item)))
        (when pp
          (when (< (width pp) (+ (offset-x item) (width item)))
            (setf (offset-x item) (- (width pp) (width item))))
          (when (< (height pp) (+ (offset-y item) (height item)))
            (setf (offset-y item) (- (height pp) (height item))))))
      (when (< (offset-x item) 0) (setf (offset-x item) 0))
      (when (< (offset-y item) 0) (setf (offset-y item) 0))
      t)))

This is awful! Am I upset? Let me tell you why.

Is it the Single Responsibility Principle (SRP)? No.

Is it Don’t Repeat Yourself (DRY)? No.

Is it Mixing Levels of Abstraction? Closer, but not quite.

Those are all clearly violated by this code. But, that’s not really the problem. The problem is Why. Nothing about this code tells you why it is here or what is doing.

There is no way to glance at that function and have any idea what’s going on. You have to read it carefully. You have to understand things that aren’t even in this source file to make head nor tail of it. Once you understand the second LET block, you will have nine more lines of code without the least inkling of why there should be nine more lines of code. Anyone care to hazard a guess as to why this function returns T (only) when we’re dragging?

Encapsulation

Two years ago, a colleague and I were tasked with providing docstrings for every function in all of the code we’d written in the last year. We’d done well on providing docstrings to the outward-facing functions, but now we had to do the rest. He started at one end of the directory (in alphabetical order), and I started at the other end. This gave me a good opportunity to look closely at a boat-load of code he’d written that I’d never really delved into before.

He was absolutely religious about encapsulating containers. If he had a hash-table or a p-list or a flat list in a DEFVAR, there was one and only one function that retrieved items from it and at most one function that added items to it. Those functions were one or two lines each (two if they needed a mutex). Those functions were named after what the collection was storing not what mechanism was used to store them.

A lot of times when people talk about the value of encapsulating, they talk about shielding the rest of the code from the implementation details so that if you need to replace how it’s actually implemented on the back end you can do it without breaking any existing code. You are protecting your precious implementation from how people will use it so that you can someday replace the implementation with an even more precious implementation next year (when your language finally gets first-class functions).

I’ve been coding for a good long time now. I’m going to let you in on a little secret. Code almost never gets replaced. When code does get replaced, it almost never continues to adhere to the old API (there was always a semantic leak). If there is a business justification strong enough to let you replace the code, it’s because the old code has become an unmaintainable mess of people subverting the interface or the code as it is didn’t scale and now synchronous things need to happen asynchronously or local things have to happen remotely and hiding that under your old API isn’t going to relieve the bottlenecks.

Trying to insulate your code so that it’s easy to replace is looking down the wrong end of the telescope. The real benefit of encapsulation is that the people who read your code later can be half-asleep and still get everything—your code will scream its meaning. The real benefit of encapsulation is that the person debugging your code can set a break-point in a place that means something—not in the seventeen places the state might have changed but in the only place it could change.

Making It Better

Any ideas what the body in this function does?

(sheeple:defreply mouse-move ((item =draggable=) xx yy)
  (if (being-dragged-p item)
      (handle-event ()
         (let ((dx (- xx (drag-starting-x item)))
               (dy (- yy (drag-starting-y item))))
           (translate-widget item dx dy)
           (keep-widget-inside-parent item)))
      (ignore-event ())))

The new functions BEING-DRAGGED-P, DRAG-STARTING-X, and DRAG-STARTING-Y are just wrappers around what had been explicitly treated as an (OR NULL (CONS INTEGER INTEGER)).

(defun being-dragged-p (item)
  (dragging item))

(defun drag-starting-x (item)
  (car (dragging item)))
(defun drag-starting-y (item)
  (cdr (dragging item)))

It is still an (OR NULL (CONS INTEGER INTEGER)) but nobody ever has to care. Nobody ever has to try to remember what the integers mean. Sure, you could replace it with a structure or a complex number, but why would you ever bother? Why would you ever look at it again?

The new macros HANDLE-EVENT and IGNORE-EVENT encapsulate the return value of this function into something with meaning.

(defmacro handle-event (() &body body)
  `(prog1
       (values t)
     ,@body))

(defmacro ignore-event (() &body body)
  `(prog1
       (values nil)
     ,@body))

It might still be too easy to write an event-handler with a path which doesn’t end in one of these two macros, but it is way better than that dangling T was. It looks like it’s really supposed to be there, and it looks like what it means rather than what it is.

The TRANSLATE-WIDGET and KEEP-WIDGET-INSIDE-PARENT functions can benefit greatly with some further helper functions (and analogous functions for top and bottom):

(defun left (item)
  (offset-x item))
(defun (setf left) (x item)
  (setf (offset-x item) x))
(defun right (item)
  (+ (left item) (width item)))
(defun (setf right) (x item)
  (setf (offset-x item) (- x (width item))))

Some Rules of Thumb

If you find that when you want to check (PRED1 ...) you instead have to check:

(and (PRED0 ...)
     (PRED1 ...))

Then you should consider making a function that does them both. Consider the difference between these two blocks of code:

(when (and (connectedp (player1 g))
           (connectedp (player2 g))
           (not (pausedp g)))
  ...)

(when (game-active-p g)
  ...)

If you find that you are depending on the NULL-ness or positiveness or some other property of some number of state variables to decide which course of action to take, then you should consider making predicates named after your state. In many OO scenarios, you may even want to explicitly track (or calculate) which state you are in at all times.

(defmacro state-case (g &body clauses)
  `(ecase (calculate-or-fetch-state-of g)
     ,@clauses))

(state-case g
 (:pause-screen-showing
  ...)
 (:settings-menu-showing
  ...))

In more imperative languages, it may even be beneficial to keep a STATE member variable in your class. When doing that, make sure that there is one and only one function which actually mutates the value of that STATE member. This will let you:

  1. Log all state transitions without having to hunt for all of them.
  2. Quickly hunt for all of them if you want to do that
  3. Set a break point on all state changes.
  4. Enforce the validity of transitions (or at least scream loudly when something transitions from STOPPED to PAUSED without having passed through PLAYING first).

If you have to check whether some resource is being used by some instance, don’t ask it which resource it is using, ask it whether it is using the one you want.

;;; Common: Reader is forced to know each player has one socket and
;;;    that sockets are comparable with #'=
(loop :for player :in all-networked-players
      :until (= socket-with-something-happening
                (player-socket player))
      :finally (return player))

;;; Better: All I wanted to know is, "Is this yours?"
(loop :for player :in all-networked-players
      :until (player-using-socket-p player
                                    socket-with-something-happening)
      :finally (return player))

Encapsulation is about protecting the person who has to read your code. It’s not about protecting your code.

Code Clarity Revisited May 11th, 2009
Patrick Stein

In an earlier post, I showed a simple loop written in several programming languages. The loop had to sum the weights of each items in a list. Dmitry pointed out a much clearer loop in Lisp using the (loop …) construct.

(loop for item in item-list sum (weight item))

I then twiddled for the better part of an hour trying to find the clearest way to do weighted random choice with Lisp (loop …). This is the best that I have come up with:

(loop
   with total = (loop for item in item-list sum (weight item))
   with thresh = (random total)
   for item in item-list
   if (minusp (decf thresh (weight item)))
     return item)

The biggest pedagogical obstacle in the above is the (decf …) which decrements the variable in place by the given amount and returns the new value of the variable. What I really wanted to do was this:

(loop
   with total = (loop for item in item-list sum (weight item))
   for item in item-list
   tracking item
   as thresh downfrom (random total) by (weight item))

That fails on multiple fronts, however.

  • There is no tracking keyword in (loop …). I can sum or maximize or collect items, but I cannot keep track of the last one that I saw. I had hoped to use the finally keyword, but its value isn’t returned from the loop.
  • Lisp requires that the decrement amount in the by of a downfrom be greater than zero. As such, I have to filter out any items that have zero weight. Feh. I can do that. I would rather not. But, I can do that.
  • Lisp only evaluates the by expression once. It does not evaluate it each time through the loop.

I am still learning all of the ins and outs of (loop …). Today, I learned as many outs as ins. Feh.

Move over UML, I Have SQL May 8th, 2009
Patrick Stein

I have been trying to nail down a design for a sizable software project. I know how I want the system to behave. I just hadn’t nailed down a good API or internal structure for it yet.

I knew that I was going to have to track multiple versions of uniquely identifiable data with different versions stored in different locations.

I had all sorts of UML drawing on whiteboards and graph paper and in OmniGraffle. Everything just kept getting messy.

Earlier this week, I read a great article about Using SQLite on the iPhone. So, today, I tried to hammer out what I would do with this data if I were using SQL to track it. I wrote a bunch of CREATE TABLE statements and then a few SELECT statements.

It worked. Getting the JOINs in the SELECT statements to make sense forced me to make the relations in my data as simple as possible, but no simpler. I had been trying to jam many-to-many relationships into one-to-many or many-to-one relationships. The SQL exercise forced me to get it right.

Will I use SQL in the final project? Maybe, but it doesn’t seem like I will need it. Did I need to use SQL for the design? Absolutely.

Updates In Email

Email:

l