Nil Object in Reality Land

Recently I was reviewing some code with a colleague; we were looking at some new widget code.  We switched to the branch and started our server.  Looking at the first randomly selected project, everything looked good, so we copied the widget code and took a look at what the user would actually see.

Problemo.  The user was going to see a 500 message through the tiny view port of the widget.

The problem was that there was code looking for the filename of the project logo in order to display the logo.  However, this particular project did not have a logo.  So, project.logo (which was nil) caused a problem when the code then sought to access project.logo.thumbnail.  Boom.  No method "thumbnail" on NilObject.  heh.

Looking at the code, the obvious pattern would have been to do something like:

if project.logo && project.logo.thumbnail

But this is cowardly, timid code and I have been influenced by Avdi Grimm's thoughts on Confident Code. Once it was pointed out, it was clear that I was never very enamored with all those protective "if this and if that and if the other thing, then and only then do something".  I really like the idea of writing code that knows how to handle itself.

My colleague and I decided to address the issue by using a NullObject pattern. Examining the code, we saw that we could have a NullLogo object that responded to a "public_filename" method and if the project had no logo, it would return the NullLogo object instead of a Logo object.  The NullLogo#public_filename method returns "no_logo.png" and so we are always guaranteed to show something reasonable.

We eliminated quite a number of lines of timid code and were very pleased.

So we pushed our code onto the staging server and looked.  Yes, the empty logo behavior was correct. So I deleted a logo from an existing project, something that can easily be done by our users.  Boom.

Digging in we saw that the Logo derived from Attachment and Attachment did magic things.  Including using a gem that managed file sizing and thumbnail generation and pushing logos to S3.  A former colleague implemented clever logic to put files in the local filesystem during development and on S3 during production; all quite reasonable.

But, deleting the logo did not trigger the cascade of cleanup; the author of the gem had no support for cleaning up attachments and all the thumbnails it had created.  Now the database had data about attachments that did not exist and S3 had files of logos that no one would ever see or even want again.

It turns out that this is the behavior that has been present in the system for ages.  Our use of TDD and careful testing revealed this problem that no one has notice although this functionality has been in the product for a long time.

To wrap up the story; we implemented functionality to clean up the database and left it as a future exercise to clean up the logos from S3.  After all, these are a tiny fraction of the storage we use there, so let's not get side tracked by the bicycle shed when we have a nuclear reactor to build.