RailsConf 2016 - Get a Whiff of This by Sandi Metz

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.