Hey, I am Sandi Metz and I am the
last but one speaker and that means
you have almost made it.
[audience laughter]
You get a break, there's another keynote
if you stay for that –
it's really bright up here, I'm going to have
to look at you some. So, yeah,
you've almost arrived. Even those of you who
are new have almost survived a total RailsConf
and so I think we should – let's do this –
sponsors pay the bills, they do, we're glad. [clapping]
Thank 'em.
But even more than that
"human people" organize this conference
and if you see – this is your job before you
leave today – find someone in one of those
red shirts, and say thank you.
Alright? Let's thank 'em here.
Thank 'em in person.
Going to start my clock, I have about –
I really do have about 40 minutes.
I don't have 500 slides of code this year,
though, so… it's not going to be that bad.
Yeah, let's just go. So
I'm Sandi Metz and I'm really happy to be here
and I have a talk. This years talk is
about code smells.
This is a term invented by this guy. So
it's really, uh, we did not plan that
I would follow Katrina
but it's the perfect talk to follow her
talk. She just mentioned this guy's name.
This guy's name is Kent Beck. This guy's
Martin Fowler, he's the man who wrote
this book, which is the book
that teaches us to do refactoring.
They collaborated together on the
third chapter of that book, and
it's like naming things wins and
the name "code smell", the reason why
you know that term today is because
of what these guys did back in the 1990s.
Now,
I wrote a book a couple of years ago
– before that, I wrote code for 35 years,
I went to my desk every day and
wrote code – and now, I don't do that.
I teach. I teach classes in
object-oriented design.
And in my classes,
I have occasion to ask people
if they've heard of code smells
and just like I suspect all of you
everyone in my class always says
"Oh yeah, we know what code smells
are." And then I ask them
to list five…
and no one can.
Alright, okay, now I can hear
you trying, but really most of you cannot.
I know that, okay.
And so, we all think
we know about this term
but it doesn't mean "I don't like your
code and I can't tell you why."
That is not what a code smell is.
There is some opinion involved
but really, it's not just that I
disagree with how you wrote your code,
the standard is higher. These smells –
these 22 different smells all have
very precise meanings…
and the power, the magic if you will
of this list is they've given things
names. Once you've given a complex idea
a name, if we could just
learn what that name stood for
it means we could just talk to each other
in an unambiguous way without
having miscommunications. And so
one of the things they talk about –
very often when people talk about
code smells, they prefix "code smell"
with the word "bad".
They say "bad smells".
But really, the definition of a code
smell is that it might indicate a problem.
You don't have an obligation
to filter out all the smells,
and it's actually important that you not.
It's also worth getting familiar
with code smells because they have
such great names.
Look at this list.
"Feature Envy" – that's cool.
I like this one, the one that
needs a code of conduct:
"Innappropriate Intimacy".
"Shotgun Surgery", that's another
favourite.
And so, the problem with this list
– first of all, okay, I'll confess –
I had that book for a really long time
before I read it. I had a really
hard time with it.
It's one of those books that's
like a recipe book and I've heard
people who – it would be like reading
a cookbook if you did not eat.
And so I really need the story
arc and I had very hard time
following along, like, persisting
with a bunch of recipes that were just
like etc., etc., etc.
But it turns out
it was really worthwhile
and I finally did it and actually
Katrina made me. I have to say it.
Katrina made me.
So there's 22 things on this list.
I had a Stack Overflow at 22.
It turns out there's a guy who's name
I can't pronounce – "Mäntylä" – it'll
be in the credits, he wrote a paper
where he grouped 'em. He grouped
'em in five different categories,
and through the magic of keynote,
I can do that.
This is the only reason we make
talks, so we can use the effects.
So here, let's just talk about
of things that just do not need
to be that big.
Long methods and large classes
are probably self-explanatory.
Data Clumps is where you have
two or more pieces of data that
always appear together; pass 'em
around and in together.
Long parameter list is obvious.
It might only occur once, but if
it's long enough, there's probably an
object in there somewhere.
And this other wonderfully named
thing called "Primitive Obsession", we saw
an example of that in the talk Katrina
just gave. This is like when you have
an instance of a base class
like a String, or a Number, or a Hash
or an Array, and you pass it
around to a bunch of objects
and they look at it and decide on what
to do based on something that
they know about it.
So you say, "I got a number,
it's six so I'll do thing X" or
"it's eight, I'll do something else."
If only the thing you got was
smarter, you could just send
it messages, so Primitive Obsession
is when the objects are too dumb
that you're passing around.
These things are grouped into
a category called "Bloaters".
They make code bigger than it needs to be
in the places where they use them.
The next group, this one,
these are ideas that are available in
object-oriented programming that you
can misuse.
Switch statements, you know they're
conditionals in normal people talk.
"Refused Bequest" is when it's an
inheritance problem. You have a subclass
that overrides a method that it
inherits from a superclass, and
throws an exception and says
like "I don't implement that thing."
"I refuse the bequest."
"Alternative Classes with Alternative
Interfaces" is pretty obvious.
The other thing is the Temporary Field –
it's interesting that Temporary Field is
on this list, right? Temporary Fields can
be really handy, but sometimes
they mean that you should have
made a method with that name, right?
Why are you giving it a name?
What is it about the code that you have
now that feels like it needs that name?
And so these things are all grouped
in a category that he called "Tool
Abusers". I work on a lot of
bikes, I have a garage full of bikes
and I'm an amateur mechanic
which means I have amateur tools
which sometimes involves a very short
wrench, a very long pipe, and a
hammer.
I can tell you that it almost always
turns out badly if you abuse your tools.
Alright, next. This group
this is stuff that makes change hard.
So "Divergent Change", "Shotgun
Surgery" – which we've talked about –
"Parallel Inheritance Heirarchies"
which is pretty obvious – sometimes
you have a couple of heirarchies
that each have two sides, and
every time you change something
you gotta go add or move
in both sides of the heirarchy.
These are the kind of things
that keep you from wanting
to change code or make code
hard to change. Now, notice that
almost everything that I'm talking
about… if nothing ever changes,
it's probably okay. Like, code that's
– really embarrasing code –
it's fine to keep that really embarrasing
code, you should be brave about
your ugly, embarrasing code
because it is not costing you money
if it's not changing. And so,
just because these smells exist,
you know, sometimes you should
just, like, own 'em. Be proud.
Walk away.
Don't let people make fun of you
for having bad code.
This next category…
"Lazy Class", "Speculative Generality" –
okay, on top of the effects
finding the pictures is also fun –
classes that don't do enough is a
lazy class, it doesn't justify its
existence. I'm going to skip Speculative
Generality for a minute. Data Class,
you know, we're object-oriented
programmers, classes oughta have data
and behaviour. Duplicated Code is
pretty obvious. Let me go back to
Speculative Generality.
I'm going to ask you to raise your hands
again. Who in here has ever
written some code for a feature you
thought might arrive in the future.
Keep your hands up for a minute.
Alright, I'm going to out myself
with you here.
Who in here has ever, after many
months of working around that code
ripped it out and thrown it
away? chuckles Yeah, okay.
We are bad guessers, and
you know, I love OO,
I love object-oriented design,
it's a thing that really interests me,
but this thing of Speculative Generality,
where we say, I'm going to do something
really cool in my code for some feature
I think we might need later…
this is why people say bad things
about OO. Right? This is what
they blame us for. It's primarily
things in that category.
You have to be right. The few times
that you are right have to really
be big wins that way the enormous
cost of being wrong – code is read
many more times than it is written.
The reason why we cost money
is the time spent reading code.
And if you add generality, you
increase the level of abstraction
of code. Very often that means
adding levels of indirection
which humans are terrible at
and every time you look at that code
it means its harder to understand.
So we should really try to
restrain ourselves, and not speculate
about the future. When the new
requirements come in, they'll tell us
how we wish we'd written the code
and we can do it then.
Dispensables. Sorry, here.
Ah, see I did that thing with the clicker!
Dispensables, okay, you've all
seen that now so we'll move on.
So the last category here is this group.
"Feature Envy", "Inappropriate Intimacy",
"Message Chains", and "Middle Man".
Feature Envy is when I have an object
uh, Joe down here is an object,
Joe's an object that I know about
and I send him way more messages
than I send myself. Could be
that I'm more tightly coupled to
Joe than I know, right?
Inappropriate Intimacy would be
when Joe had a bunch of private
methods and I reached in and
got them, okay, that would be bad.
laughs I really am sorta pushing
the code of conduct, aren't I?
I hope no-one was made uncomfortable
by that. Message Chains – there's
the Law of Demeter, those violations
right? You got dots. You send
messages to something you know about
and you get a response and you send
a message to that. If the types
change across those dot changes
that's a Message Chain.
Middle Man is if you have
an object that you send messages to
and it's sole purpose in life is to
forward those messages to somebody else
maybe you don't need that object.
These things are grouped together
in a group called "Couplers"
because the effect of this
is that it binds the objects together
such that even if they're
beautiful, even if you've tested
'em, even if they're little
works of art, you can't ever
reach in and get one out
and use it in another context.
They come as a bundle, all or nothing.
And so there you go.
10 minutes and 34 seconds
everything you need to know about
code smells.
Okay, but we're not done.
Now I'm going to talk about refactoring
but not very much
because you just heard a talk
on refactoring.
But there's a thing here
that people don't know,
that those guys discovered in the 90s,
that I want you to go away today
understanding, and it's this:
refactorings, just like code smells have
names and they mean very specific
things, refactorings have names
they're very specific, and they
come with recipes. Not hand-wavy
recipes, very, very specific, concrete
recipes. Here's one.
This is page 149 of Martin Fowler's
book, and it looks like a recipe, right?
It has numbers down the side.
There's little optional clauses here
for situations that might be different in
your case. You notice that it refers
to other recipes, here where the
things are in capital letters, that's
another whole recipe by itself.
It's recipes and recipes
within recipes.
All of the refactorings work in the
same way. This is not something –
we don't wave our hands and say
"go refactor" – refactoring has a
very specific definition, it's to
rearrange code without changing its
behaviour and all the ways
in which you can rearrange code are
already written down, with instructions
by people who really thought a lot
about this. And so now you know
code smells have names, and they're
real things, and refactorings have names
and they're real things, and they
come with recipes. I can give you
one last bit of news. Every code smell
maps to the Curative Refactoring Recipe.
Ponder that for a minute.
What does that mean?
Here's a cheat sheet. This one's
provided by the guys at Industrial
Logic. Notice at the top that the code
smell they're talking about is Data
Clumps. This is a little tiny definition
of a Data Clump. "[F 81]" is a
reference to page 81 in Martin
Fowler's book. The three things
on the bottom are the
refactoring recipes that are curative
for that code smell.
So this slide, I just blew it up
so you could see it,
I just extracted it from this PDF
which is a couple pages long. It
cross-references all the code smells
and refactorings in Martin Fowler's
book and also a book by
another guy named Joshua Kerievsky
called Refactoring to Patterns.
It turns out that this is all
you need to know. The problem
is solved. You do not have to
reinvent this wheel.
All you need to know is a few things.
And many of you –
at least, my experience –
the reason I wanted to give this talk
from my experience teaching is
somehow a generation has passed
since all these books were written, and
that people new to programming
in the last 15 years –
I'm a woman of a certain age,
you can tell –
if you're new in the last 15 years
you may not have this information.
I'm going to show you some code now.
I'm going to use the last 15-20 minutes
looking at code, and I'm going to
show you the practical effect of
recognizing code smells and doing
refactorings.
My class Sale is a subclass of
Persistence. You can think of that as
ActiveRecord. If you're in the back it
won't hurt my feelings if you get up
and come forward – that's the font
size of my code.
I didn't know I was gonna be in the
keynote, man, it just happened.
Let's say I have my class Foo that has
a sales_total method, takes some params,
and maybe this a controller-like thing
or something that a controller calls.
It knows the name of the Sale class and
it knows some other things, right?
It knows that Sale understands where
and it knows that the thing that comes
back as a response from sending the where
message knows sum.