Don't extract that method yet!

In programming, we tend to work with collections of objects quite often. For example, we may need to check to see if a user is in a collection of users, or display all of the achievements a user has. Ruby, Rails and ActiveRecord provide some interesting ways to make that easier. For example, if we have a collection of achievements for a user, we can specify:

class Achievement
end

class User
		has_many :achievements
end

And then do things like

 user.achievements << Achievement.new("Blah")

or loop over them. And when we are looping over them, we can do all kinds of fun things:

#user has achievements "Code Monkey", "Works on My Machine" and "Get off my lawn"
achievements = user.achievements

achievements.map { |a| a.upcase }
#returns "CODE MONKEY", "WORKS ON MY MACHINE", etc

achievements.find { |a| a.contains?("lawn") }
#returns "Get off my lawn"

achievements.join(',')
#returns "Code Monkey, Works on My Machine, Get off my lawn"

These capabilities are quite powerful - until your code is littered with them everywhere. Views are looping over collections. Controllers are modifying and limiting them. Models are transforming them. And before you know it, you have to change some aspect about it, and find a subtle bug because you did the transformation in several places, and didn't change them all.

(Maybe we need a new law: For any set of duplicated code n you will find at most n-1 when the code needs to change)

So, following the Once and Only Once rule, we dutifully extract them into the place that seems to make sense - the Achievement model:

class Achievement
		def self.all_upper
				return Achievement.all.map { |a| a.upcase }
		end

		def self.as_csv
				return Achievement.all.join(',')
		end
end

And this is good because the code is in one place, but it is very bad too. It's bad because we are hiding an abstraction that is begging at us to come out, quashing and squishing and shoving it into a place it doesn't belong. That abstraction? Achievements

There was a running joke when I first started with 8th Light that you could tell a project I was on because you would suddenly see a class like this:

class Achievements < Array
end

What people called quite bizarre I called "Explicit Collections". The purpose was to drag the abstraction we were using out of the model class and into its own proper element. Now we could do things like:

class Achievements < Array

		def to_csv
				self.join(',')
		end

		def to_upper
				self.map { |achievement| achievement.upcase }
		end

end

They key here is this isn't about brevity. This is about clarity. When you are building systems, you have to watch very closely what it is telling you. And when you shove everything into a cabinet and lock it up, you may miss very important information about what it's trying to tell you. By introducing explicit interfaces such as this, you instead let the idea run around screaming, reminding you each time you use it that perhaps there's something else about the system that you are missing.

The lack of screaming from our code is one of the key challenges in building great code iteratively. If you force everything into a cabinet before it's ready, you might miss what it is really trying to tell you about how the system should look. And the more you suppress, the more your cabinets become filled, until you find yourself having trouble finding space for things.

Instead, if you don't know where something goes, don't just shove it in a cabinet, or in an abstraction. Set it out on the counter. And when you get two or three, you'll have a much better idea of where they go.

So the next time you find yourself refactoring some action to a method, ask yourself if that extract refactoring is actually a part of the class you are refactoring to, or is it its own concept worthy of making explicit to the world? You might just find your cabinets - and your code - a lot cleaner over the long term.