Sandi Metz's Blog

My 2011 GoRuCo talk, Less - The Path to Better Design, is at Vimeo.

It's mostly about reducing object coupling and is full of example code. If you occasionally find yourself forced into a corner where you have to write case statements that switch on class, have a look, this will help.

Plus, the last two and a half minutes are full of bicycles; what's not to like?

The slides are on Heroku.

Thanks again to all the GoRuCo organizers. The conference is great and is an accurate reflection of their commitment to the community.

My GoRuCo talk on SOLID Object-Oriented Design has been up on Confreaks for a while. They did a great job recording all the talks; very professional.

You're an object - Stand up straight and act like one!
Imagine you have this code:

Code 1 1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
class MyView
attr_reader :target

def initialize(target)
@target = target
end

def double
case target
when Numeric then target * 2
when String then target.next # lazy example fail
when Array then target.collect {|o| MyView.new(o).double}
else
raise "don't know how to double #{target.class} #{target.inspect}"
end
end
end

It does just what you'd expect.

Output 1 1
2
3
4
5
6
7
8
9
10
11
>> MyView.new('aaa').double
=> "aab"
>> MyView.new(49).double
=> 98
>> MyView.new(['b', 6]).double
=> ["c", 12]
>> MyView.new({'x'=>'y'}).double
RuntimeError: don't know how to double Hash {"x"=>"y"}
from (irb):73:in `double'
from (irb):80
from :0

You're probably familiar with this pattern. Its everywhere in Rails and you likely use it in your own code.

I want to say, in the nicest possible way, that this style of code is wrong, wrong, wrong and that you should do a different thing.

Okay, now that I have your attention, I'm not trying to start a fight. I'm not the best Ruby person around and I'm definitely not the best OO designer, but I do have an alternative pattern to suggest.

I'm aiming to start a discussion, not a religious war. Strong opinions are welcome.
What's happening up there?MyView needs to operate on several other objects.  It knows:the classes of all the objects that it can interact with, andthe behavior that should occur for each of those classes.The case statement above is really an 'if' statement that checks 'kind_of?' on each collaborating object.

I object to this code because:use of 'kind_of?' is a code smell that says your code is procedural, not object oriented, andif you write procedural code your application will gradually become impossible to change and everyone will hate you.Why is it wrong?If I change how 'double' works on any of these classes, MyView must change, but that's not the real problem.  What happens if MyView wants to double some new kind of object? I have to go into MyView and add a new branch to the case statement.  How annoying is that?

But that's the least of it.  If I'm writing code that follows this pattern, I likely have many classes that do stuff based on the classes of their collaborators.  My entire application behaves this way.  Every time I add a new collaborating object I have to go change code everywhere.  Each subsequent change makes things worse.  My application is a teetering house of cards and eventually it will come tumbling down.

Also, what if someone else wants to use MyView with their new SuperDuper object? They can't reuse MyView without changing it since MyView has a very limited notion of what kinds of objects can be doubled.

MyView is both rigid and closed for extension.
What should I have done instead?Something like this.

Code 2 1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
class Numeric
def double
self * 2
end
end

class String
def double
self.next
end
end

class Array
def double
collect {|e| e.double}
end
end

class Object
def double
raise "don't know how to double #{self.class} #{self.inspect}"
end
end

class MyView
attr_reader :target

def initialize(target)
@target = target
end

def double
target.double
end
end

Using this new code, Output 1 will be the same as before, but now we can also:

Output 2 1
2
3
4
5
6
>> 'aaa'.double
=> "aab"
>> 49.double
=> 98
>> ['b', 6].double
=> ["c", 12]
In this example, objects are what they are and because of that they behave the way they do.

That statement is deceptively simple but incredibly important.  Objects ARE what they are so they do what they do.

It is not the job on any object to tell any other object how to behave.  Objects create behavior by passing messages back and forth.  They tell each other WHAT, not HOW.  

WHAT is the event/notification/request and it is the responsibility of the sender.

HOW is the behavior/implementation and it should be completely hidden in the receiver.

Code 2 is object oriented because it relies on a network of interacting objects to produce behavior.  Each object knows its own implementation and it exhibits that behavior when it receives a message.

No object in your system should have to know the class of any other object in order to know how to behave.  Everything is a Duck.  Tell the Duck WHAT and the Duck should know HOW.

This way you can change how any object is doubled by changing its own specific implementation.  More importantly, MyView can now collaborate with any kind of object, as long as the object implements 'double'.
This is a nice theory, but it seems impossible in practice.Nah, it's easy.  But it means thinking about objects in a more Object Oriented way.

In order to write code like Code 2, you have to believe in objects.

In Ruby, everything is an object.  I know that you know this, but I suspect that you don't feel it in your bones.  You came from those languages where you couldn't change String so now you operate as if String (and Symbol and Array and ...) are static types.

Throw off your shackles.  Ruby is a network of interacting objects and you can add behavior to all of them.  When you find yourself saying, if you're this kind of thing, do this, but if you're that kind of thing, do that, it's your cue to push the behavior back into those individual objects.  
Sound bites.Always send messages if you can.  
Implement behavior only when there's no one left to ask.  
The last object that could possibly receive the message is the object that should implement the behavior.
Therefore:MyView should not know how Numeric implements 'double'.
MyView should just ask target to double itself.
All possible targets must implement 'double'.
Object should implement 'double' to help you get your Ducks in a row.
Lest you think this is all academic...Here's the code to use this Pattern in some of the form_for methods in ActionView::Helpers.  Ponder the implications.
Diff of the changes.
actionpack/lib/action_view/helpers/form_helper.rb
actionpack/lib/action_view/helpers/prototype_helper.rb
actionpack/lib/action_view/helpers/form_helper_core_extensions.rb

Not as Painful as it sounds...Nothing is more pleasing than beautiful code.  And nothing is more heart-breaking than watching beautiful code get destroyed.

Lately, I've been paying particular attention to SOLID Object Oriented Design (OOD) principles and their interaction with TDD.  I'm finding that, while TDD is an essential first step, it just isn't enough.  If I want my code to survive the rigors of change and be useful for a long time I need to armor it by following SOLID principles.

There's a delightful symmetry in the feedback loop between TDD and OOD.  TDD in isolation is not guaranteed to produce well designed code, but if you follow the OOD principles while writing the code you're testing, TDD gets easier and your tests get better.

In case you feel like pushing back already, let me add an early caveat.  If youhave an extremely simple applicationwith a specification that's 100% completethat will never ever changego on ahead and write it any way you'd like.  It doesn't matter.

I say good luck with that.  Let me know how it works out for you.

But if you're living in my reality, have a listen to Uncle Bob.  In Design Principles and Design Patterns he describes good software as

'clean, elegant, and compelling', with 'a simple beauty that makes the designers and implementers itch to see it working'.

But then goes on to say:

What goes wrong with software?
The software starts to rot. At first it isn’t so bad. An ugly wart here, a clumsy hack there, but the beauty of the design still shows through. Yet, over time as the rotting continues, the ugly festering sores and boils accumulate until they dominate the design of the application. The program becomes a festering mass of code that the developers find increasingly hard to maintain.

There's more, but if I told you all of it I'd have to send you to the dermatologist.

If you have good tests, you can protect the reliability of any smelly pile of software, even if it makes you cry when you have to change the code.  $$'s fly out the window but it all still works.

If you have good tests AND good design, the software will be reliable and changes will be an economical pleasure.  You'll look forward to adding new features so you can undertake big refactorings and make the code do even more.

But, enough with all this talk.  Let's do something.

Dependency Injection

Bob [1] calls it Dependency Inversion and you could definitely argue that the two concepts are slightly different, but don't quibble with me about this.  I'm being practical here.

Example 1:1
2
3
4
5
6
7
8
9
10
11
12
13

class Job
def run
@retriever = FileRetriever.new
strm = @retriever.get_file('theirs')

@cleaner = FileCleaner.new
cleaned = @cleaner.clean(strm)

local = 'mine'
File.open(local, 'w') {|f| f.write(cleaned) }
local
end
end
Class Job is responsible for retrieving a remote file, cleaning it up and then storing it locally.  It uses two preexisting classes, FileRetriever and FileCleaner, which themselves have thorough tests.

The Job class is dirt simple.  If you wrote it test first, you might have a spec like:1
2
3
4
5

it "should retrieve 'theirs' and store it locally" do
@job = Job.new
local_fn = @job.run
local_fn.should have_the_correct_contents
end

What does this spec test?  Job?  Or Job AND FileRetriever AND FileCleaner?  Obviously, all three.  My spec is testing a whole set of objects; Job and all of it's dependencies.  It's fragile in that it relies on objects other than the one under test and it runs too long because it exercises code that it should not care about.

Mocks/stubs to the rescue, right?  I could stub FileRetriever.get_file and FileCleaner.clean and bypass both of those problems. However, even if I stub those methods, my code still has a bad smell.  Stubbing improves the test but does not fix the flaw in the code.

Because of the style of coding in Job, it contains dependencies that effect my ability to refactor and reuse it in the future.  Let's move some code around.

Example 2:1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

class Job
attr_reader :retriever, :cleaner, :remote, :local

def initialize(retriever=FileRetriever.new, cleaner=FileCleaner.new,
remote='theirs', local='mine')
@retriever = retriever
@cleaner = cleaner
@remote = remote
@local = local
end

def run
strm = retriever.get_file(remote)
cleaned = cleaner.clean(strm)

File.open(local, 'w') {|f| f.write(cleaned) }
local
end
end

Now I'm injecting the dependencies into Job.  Suddenly, Job feels a lot less specific and a lot more re-usable.  In my spec I can create true mock objects and inject them; I don't have to stub methods into existing classes.

That stylistic change helped a lot, but what if I want to provide some, but not all, of the arguments? It's easy, just change the initialize method.  It wouldn't bother me if you also wanted to simplify run.

Example 3:1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

class Job
attr_reader :retriever, :cleaner, :remote, :local
def initialize(opts)
@retriever = opts[:retriever] ||= FileRetriever.new
@cleaner = opts[:cleaner] ||= FileCleaner.new
@remote = opts[:remote] ||= 'theirs'
@local = opts[:local] ||= 'mine'
end

def run
File.open(local, 'w') {|f|
f.write(cleaner.clean(retriever.get_file(remote)))}
local
end
end
That feels really different from example 1. A simple change in coding style made Job more extensible, more reusable and much easier to test.   You can write code in this style for no extra cost, so why not? It will save someone pain later.

Example 4 - Pain:
Here's some code from Rails that generates xml for ActiveRecord objects.  (Please, I'm not picking on them, this just happens to be a good example that I dealt with recently.)1
2
3
4
5
6
7
8
9

module ActiveRecord #:nodoc:
module Serialization
def to_xml(options = {}, &block)
serializer = XmlSerializer.new(self, options)
block_given? ? serializer.to_s(&block) : serializer.to_s
end
#...
end
end
Without recounting the whole story, I wanted to use to_xml with a different Serializer class.  Imagine how easy it would be if XmlSerializer had been injected into to_xml.  Instead, look at it and despair.  I have to override the entire method just to name a different serializer class, with all the future maintenance burden that the change entails.

The to_xml code does exactly what it's supposed to do and in that way cannot be faulted.  The person who wrote it isn't bad, they just never imagined that I would want to reuse it this way.

Let me repeat that.

They never imagined how I would reuse their code.

The moral of this story?  The same thing is true for every bit of code you write.  The future is uncertain and the only way to plan for it is to acknowledge that uncertainty.  You do not know what will happen; hedge your bets, inject your dependencies.

TDD makes the world go 'round.  It lets us make unanticipated changes with confidence that our code will still work, but SOLID design principles keep the code 'clean, elegant, and compelling' after many generations of change.

Notes:
1) I don't mean to be overly familiar; it's not like I know the man.  But he's an icon, how can I avoid calling him 'Bob'?

How was the Voices That Matter Professional Ruby Conference?

It was a cross between being thrown in a geek-fest mosh pit and putting on a sweater that was warm from the dryer.

The Ruby crowd is a fierce meritocracy where everyone is scary smart and each believes that everyone else is slightly smarter. They're chasing perfection and claiming ignorance. They love a good idea. They want to change the world.

The conference was 96% men.

There may be places in IT where women aren't welcome but, despite that disparity, this ain't one of them.

I ended up there with a speaker flag on my badge and it's remarkable how many men stopped me to say how much they missed us and how they wanted us both in the seats beside them and on the podium in front of them.

I'm inspired to stand up and be counted. (Short though I am).

But seriously, where is everyone? There was a time when I had to wait in line in the women's room at conferences and now it echoes in there.

While you ponder this question, you can get an actual tech update of the conference at Liana's Wrap Up and Obie's Summary.