Relative Sanity

a journal

For instance

7 January, 2015

I often see (and occasionally write) code like this:

class Profile

  def initialize(user)
    @user = user
    @data = ProfileData.for_user(user)
  end

  def address
    [@data.street, @data.town].compact.join "\n"
  end

  def name
    [@user.first_name, @user.last_name].compact.join " "
  end

end

Now there's nothing wrong with this code. It's perfectly servicable. It could definitely be tidier, but it's not terrible code.

What it is, however, is "refactor-resistent" code. Let's say ProfileData is issuing enquiries over the network to some third party service. If it turns out we very rarely use the address method, we're making that expensive call to the ProfileData source needlessly every time we want to build the user's name. We refactor this to be lazy loaded:

class Profile

  def initialize(user)
    @user = user
  end

  def address
    [data.street, data.town].compact.join "\n"
  end

  def name
    [@user.first_name, @user.last_name].compact.join " "
  end

private

  def data
    @data ||= ProfileData.for_user(@user)
  end

end

Great, now we only load the data when we need it, but look at the other changes we needed to make: the whole of the address method changed, despite nothing about the way it worked changing at all.

This is because the address method didn't just know about the object's data, it also knew that we were storing it as an instance variable. That meant when we changed that fact, everything else that knew about the fact had to be changed.

We could have saved ourselves this problem by using attr_reader in the first place:

class Profile

  attr_reader :data

  def initialize(user)
    @user = user
    @data = ProfileData.for_user(user)
  end

  def address
    [data.street, data.town].compact.join "\n"
  end

  def name
    [@user.first_name, @user.last_name].compact.join " "
  end

end

attr_reader :foo simply defines a method foo which will return the value of any @foo instance variable assigned by the object. With this in place, our subsequent refactor doesn't touch any methods that aren't directly related to the use of ProfileData:

class Profile
  
  def initialize(user)
    @user = user
  end

  def address
    [data.street, data.town].compact.join "\n"
  end

  def name
    [@user.first_name, @user.last_name].compact.join " "
  end

private

  def data
    @data ||= ProfileData.for_user(@user)
  end

end

Those of you paying attention will have noticed that the attr_reader version is slightly different from the rest: it exposes a public Profile#data method which can be read by the outside world. This might not be good, and is certainly a change in interface. No worries:

class Profile

  def initialize(user)
    @user = user
    @data = ProfileData.for_user(user)
  end

  def address
    [data.street, data.town].compact.join "\n"
  end

  def name
    [@user.first_name, @user.last_name].compact.join " "
  end

private

  attr_reader :data

end

All is well once again. We can complete this by performing the same up-front trick with @user:

class Profile

  def initialize(user)
    @user = user
    @data = ProfileData.for_user(user)
  end

  def address
    [data.street, data.town].compact.join "\n"
  end

  def name
    [user.first_name, user.last_name].compact.join " "
  end

private

  attr_reader :data, :user

end

and now if we ever change how @user gets set (say switching to take a user_id as the initializer's argument), nothing else needs to care.

Encapsulation isn't just good practice for external interfaces — its benefits can be felt right down to the object internals and method level.