Code to an Interface (aka Stop Using Instance Variables)

We all know the drill: only call methods a class declares public, leave protected and private methods alone, because they can change at any time. In other words, code to the public interface and don't depend on implementation details. It keeps our code clean and means that when the internals of a class change, its clients don't have to.

Curiously, we rarely apply the same thinking when managing state inside our own classes — and that can make refactoring surprisingly painful.

The problem with bare instance variables

Here's a Book class from a hypothetical bookstore application. Books have titles and authors. They have a publication date that can change — maybe the author misses a deadline, or editing runs long. Titles can change too, but authors won't.

class Book
  attr_reader :author
  attr_accessor :title, :published_at

  def initialize author, title, published_at
    @author = author
    @title = title
    @published_at = published_at
  end

  def to_s
    "\"#{@title}\" by #{@author}. Publication date: #{@published_at}"
  end
end

A few weeks pass and we start doing more deals with publishers. One of them wants us to exclusively list an upcoming book by A.N. Big Author. Great! Except… we can't handle books that don't have a publication date yet. We need to update the class:

class Book
  attr_reader :author
  attr_accessor :title, :published_at

  # published_at = nil if the book doesn't have a publication date
  def initialize author, title, published_at
    @author = author
    @title = title
    @published_at = published_at
  end

  def to_s
    "\"#{@title}\" by #{@author}. Publication date: #{@published_at ? @published_at : 'not yet published'}"
  end
end

That's tolerable for this tiny class, but it's ugly, and it's easy to imagine a real class where @published_at gets accessed directly in a dozen places. Changing every one of those takes time and the resulting conditionals don't read well. It's a prime candidate for the Introduce Null Object refactoring, but because we're reaching for @published_at directly everywhere, there's still a lot of churn. We could introduce the Null Object during instantiation, except the publication date can change at any time — a publisher might call and say they've missed their date and don't know when they'll publish.

A better starting point

Here's the class I wish I'd written from the beginning. It exposes the same public API but uses accessor methods internally instead of bare instance variables:

class Book
  attr_accessor :author, :title, :published_at
  private :author=

  def initialize author, title, published_at
    self.author = author
    self.title = title
    self.published_at = published_at
  end

  def to_s
    "\"#{title}\" by #{author}. Publication date: #{published_at}"
  end
end

Now when I get the call about the unpublished book, I can introduce a Null Object by simply overriding the reader for published_at:

class MissingPublicationDate
  include Singleton
  def to_s
    'not yet published'
  end
end

class Book
  attr_accessor :author, :title, :published_at
  private :author=

  def initialize author, title, published_at
    self.author = author
    self.title = title
    self.published_at = published_at
  end

  def published_at_with_null_object
    published_at_without_null_object || MissingPublicationDate.instance
  end
  alias_method :published_at_without_null_object, :published_at
  alias_method :published_at, :published_at_with_null_object

  def to_s
    "\"#{title}\" by #{author}. Publication date: #{published_at}"
  end
end

It's a touch more code in this example, but almost none of the methods that use published_at need to change, and the result is vastly more readable. The lesson: treat your own class's state the same way you'd treat someone else's API. Code to the interface, even internally.