a journal
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 initialize
r'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.