00:00:15.639
how's everybody doing yeah nice crowd you guys look good
00:00:20.880
we got people packed in the back can you guys do me a favor this I think this is the biggest crowd that's ever turned out for a talk of mine so will you do me a
00:00:26.400
favor and wave hi to my mom you guys are great Sports thank you so
00:00:32.719
much so my name is Ben orstein I work what's that oh it'll be oh yeah
00:00:38.160
absolutely after the talk so my name is Ben orstein I work at thoughtbot in Boston and this talk is about
00:00:44.079
refactoring do we have any fans of refactoring in the audience do we have any rubyists in the audience do we have
00:00:50.280
anyone in the audience who's never raised their hand before interesting so uh just a quick little
00:00:56.680
bit of administration I want to get out of the way uh this talk is not a lecture from the clouds this is not me telling
00:01:04.080
you the one true way to do things this is not a lecture at all this is pairing
00:01:09.720
I would now like to pair program with all of you so please consider this an informal
00:01:17.000
dialogue now this is partly because I want you to understand things the purpose of this talk is not for me to
00:01:22.360
reach the end it's to teach you things but also I want you to be able to correct my mistakes when I make mistakes
00:01:28.600
in live coding so I'm going to need those backup six or 700 pairs of eyes or
00:01:33.640
single eyes so let's get started let's look at some code who likes looking at
00:01:40.439
code here's some code take a second and read
00:01:45.520
this use both your
00:01:53.799
eyes what's that do you want to try here let's have
00:01:59.439
a vote better or
00:02:09.520
worse can we have him removed please security we have a Heckler you're a bad pair
00:02:17.680
sir all right getting the gist of this we take a collection of orders we take a
00:02:23.120
start date at an end date and we're curious what the total sales were during the date range we pass in now I have
00:02:30.319
specs for this code I run them like this they're going to go by really fast because I'm not loading
00:02:35.920
rails yeah so I'll be running these tests continuously as I'm doing these refactorings much as you always do 100%
00:02:43.760
of the time when you're refactoring right excellent that's the first rule of refactoring don't refactor code that
00:02:49.360
doesn't have tests all right so looking at this code this is roughly how it would have
00:02:56.440
written this report about 18 months ago or so I probably would look at this call it
00:03:01.680
good enough and commit it or actually i' probably check it in back in some version days and uh and call it a day
00:03:09.560
but now I look at this code and I think there are some things that could be better so let's start with one check out
00:03:14.920
line 12 I have this temp variable called orders within range and I'm assigning it
00:03:20.360
the results of some calculation now the first thing I would do something I've been doing a lot lately is pulling out temp variables
00:03:27.760
into query methods so I have a macro recorded that does this don't freak out
00:03:32.959
but it's going to happen fast here's the before here's the after I've pulled that
00:03:40.519
temp into a private method with the same name and I'm just referencing it
00:03:46.640
here now why am I doing this well first of all notice despite the weird wrapping that's going on what's actually happened
00:03:52.959
here is I've gone from one method with two lines to two methods with one line
00:03:58.879
now I'm not going to tell you that that that is always 100% of the time a good idea but it's usually the right direction usually feel good usually
00:04:05.920
feels good the rule of methods is that they should just do one thing and shorter methods tend are more likely to
00:04:11.560
be doing one thing so that's nice the methods are shorter but also there's another benefit
00:04:16.919
we picked up here which is that we can reuse it so when orders within range is a temp
00:04:22.000
variable like this it's locked up inside this method when it's like this it's not
00:04:29.440
that's calculation is now available to other methods in this class and I think it's actually a little bit more likely that I'm going to reuse
00:04:36.360
this rather than duplicate it when I pull it out like that finally there's a third benefit and this actually might even be the most important
00:04:42.320
one when I pull out that calculation into a private method it's less likely
00:04:48.240
that you'll read it so when you look at code like this because we're programmers and we like to read code we see orders
00:04:54.360
within range equals and then we read what goes into that value and we figure
00:05:00.080
it out and we parse it and we run it in our heads when it looks like this you're more likely to say oh that's a private
00:05:06.120
method okay and move on and you're just going to take my word for it that orders
00:05:11.759
within range Returns the orders that are within the range so I've given you a hint right here the hint is this is an
00:05:18.880
implementation detail it's not important it's a lower level of abstraction that we're not interested in and there
00:05:24.520
another benefit pulling that out into a query method so let's take a look at where we
00:05:30.680
are now this code's getting better but it's not great so one place another
00:05:36.360
place I start to look for refactoring these days isn't what about our test good call yeah thank you still
00:05:44.000
green man this is why you have pairs because you forget stuff okay so we're still green thank
00:05:50.440
goodness so another place I look for refactorings these days is in these um semi complicated bodies of selects like
00:05:56.759
this see how you've got this andand in here that is a good hint to me that that
00:06:03.520
could be refactored that we could give a name for this and also notice what we're doing with order we're asking order
00:06:11.479
questions about itself so we're saying hey order what's your placed at when were you placed at and I'm going to do some calculations and if you if you
00:06:19.240
match these calculations I'm going to select you now I don't love this this feels a little bit like feature Envy a
00:06:25.240
feature Envy is where one class seems really interested in a different object and it's a smell it says maybe that
00:06:31.960
method that seems really interested in that other object should actually be moved you should actually move it to that class so what if rather than poking
00:06:38.440
it at order and pulling out one of its attributes what if we just had a method on order that would answer the question
00:06:44.280
that we want to have answered so here's what this might look like we could just say hey order were you
00:06:51.000
placed between the start
00:06:56.639
date and the end date what's that no Macro for this one so oh
00:07:04.759
by the I had so many people ask me in recordings how' I wrote that macro that extracted the private thing that's just like a one-off macro it's not
00:07:10.599
generalizable I wrote it just for this talk just so you guys wouldn't have to sit there and watch me fiddle with this but sadly it is not a general macro
00:07:19.000
yet how about using a range that's a perfectly good question and we're totally going to use a range object in a few steps so this guy this man is from
00:07:26.280
five minutes in the future so what happens now when we run our test well they bomb because as we'
00:07:33.080
guess order does not have a place between method so let's write one place between takes a start date and an end
00:07:40.080
date and now we have to say is our place at after the start date and is our place
00:07:47.240
that before the end date did I get those uh things right let's see I
00:07:53.360
did okay so now I think this code is looking a little better let's move this
00:07:59.000
down you can see it can I do that nope silly Ruby
00:08:05.599
uh okay so things are looking better I now like that the logic for determining
00:08:12.319
if an order is placed between two dates is an order it's it's a thing that we have order deal with rather than pulling
00:08:17.560
out pieces of order and making decisions on its behalf we're sending order a message and being like hey you worry
00:08:23.120
about this you figure out this you contain this collection that's sort of a nice or you contain this calculation and
00:08:28.919
that's a nice direction to be heading in in object oriented programming I like that this calculation is closer to where
00:08:34.800
the data is but now if you look we've introduced another smell so take a look at all the
00:08:40.959
places where we have the pair of start date and end date we pass it in the
00:08:48.279
initializer we pass it down to order and we use it in this
00:08:53.360
method so this is something that I believe Martin fer calls a data Clump and a data Clump is just one or more or
00:08:59.720
two or more parameters that you see together all the time particular in parameter lists and argument lists and a
00:09:05.680
great test to see if you have a data Clump is to try taking away one of the elements so if we had a start date and
00:09:11.120
an end date and we want to know if something's between them and I took away the start date it doesn't really makes sense anymore those things are actually
00:09:17.440
linked they're basically one concept maybe it's a range who knows but it'd be nice to take this
00:09:25.880
implicit concept which is start data and end date all together and make it an explicit concept that's something I find
00:09:32.160
myself thinking about a lot these days what what is something that's implicit in this code that I could make more
00:09:37.640
explicit because the more explicit you make the more easy it just understand the less likely it is that you're going have to explain it to somebody else
00:09:43.440
especially yourself in three months so what would this look like well when you have a data Clump the thing to
00:09:49.399
do is give it a class and stick those things together so let's take a look at what that would look like I'd like to
00:09:55.480
rather than have uh let's see how we want to do this want do this in small steps so let's start by making a data
00:10:03.200
the date range right here so I'm going to say range I'm going to create a new class
00:10:09.360
called date range I'm going to pass the start date and the end date into
00:10:16.560
it and then I'm going to feed it into here into our order place
00:10:24.839
between uh yes it's because I well yes so uh normal range will work this is for
00:10:30.279
illustrative purposes I'll talk about uh using a plane range in a second but yeah
00:10:35.399
um I think there's a slight I think there's a slight benefit to calling it date range instead of just a plain range
00:10:41.120
makes it a little more obvious but I think at the end of the day I might replace this with a range but we'll talk about this in a sec so I want to create
00:10:48.760
a new date range we have an initialized constant which we' expect here's our new date range and I'm
00:10:56.200
just going to make it take a start start date and an end date
00:11:02.560
see how we're doing now so we have the wrong number of arguments now for Place between right in here so let's make this
00:11:09.000
take a date range and now we're going to need to
00:11:18.000
call start date and end date on those on the date
00:11:24.839
range back to green so let's let's go one step further
00:11:30.600
we have this date range rather than build it in order I'd rather pass it into order I'd rather invert control and
00:11:37.720
put and inject this dependency instead so what does that look like let's open up our spec for a
00:11:43.880
second so I'm going to actually feed
00:11:50.000
in the start date and end date from the test I'm going to come in
00:11:56.360
here pass in that range into orders report this blows up because we're now passing the wrong number of arguments to ord
00:12:02.920
report so let's find that date range
00:12:10.079
here date range and now down here rather than building the date range that's already
00:12:16.000
built and we can just reference the range from here oh thank you that's what
00:12:22.519
I get for using different names all right back to green Everybody follow what we just did right there new
00:12:29.639
class it holds the start date and end date it's just packaging them up so is this a
00:12:34.680
win no no yes we have NOS we have yeses I think it's a
00:12:40.839
win yes good uh so why is the win well first
00:12:47.680
of all we've reduced coupling here so what's coupling coupling is the degree to which components in a system rely on
00:12:54.720
each other and we're interested in coupling as programmers because when two things are are extremely coupled it
00:13:00.160
becomes very hard to change one without affecting the other whereas if two things are completely decoupled you can
00:13:05.480
change one all day and never affect the other so in general in our programs lower coupling is better because we want
00:13:12.000
to make it easy to change things that's that's a really good metric for code equality how easy is this to
00:13:17.760
change so what kind of coupling existed before well there's one kind of coupling called parameter coupling let's look at
00:13:23.240
an example of that so in our top method we in an object called failure
00:13:29.959
don't worry about that doesn't not coming from anywhere and we pass it into print a console so print a console then
00:13:35.959
calls a method on failure it calls two sentence on failure now the thing to notice here is
00:13:43.560
print a console and notify user of failure are coupled to each other and they're coupled because notify user of
00:13:49.639
failure has to build the right failure object to pass into the other method if
00:13:55.279
I create an object if failure here does not respond to two sentence this method will blow up so in effect the fact that
00:14:03.160
this method calls two sentence on its parameter has leaked over to this other method they are coupled to each other
00:14:09.320
one depends on the other and less coupling is better in general so
00:14:14.480
parameter coupling is one kind of coupling the interesting thing about this is if you have a method that takes
00:14:19.759
no arguments it has no parameter coupling and that actually means that a method that takes no arguments is
00:14:26.040
superior to a method that takes one argument and those of you that are familiar with
00:14:31.240
induction will probably guess that a method that takes one argument is better than a method that takes two
00:14:36.480
arguments so it's actually worth your time often to slim down your parameter
00:14:41.759
lists it tends to make your code easier to understand but also it reduces coupling it reduces parameter coupling
00:14:48.000
not always a huge win but it's usually a good idea so now we've taken our orders report from three arguments to two by
00:14:54.720
putting this together in a date range uh also we've now got a really handy place to
00:15:01.920
hang new Behavior so notice that order plays between is answering the question hey is
00:15:09.160
some date between two other dates this seems like the kind of thing
00:15:14.839
that I'm going to want to reuse I might want to say hey was this coupon issued between two dates hey was this gift
00:15:20.480
certificate redeemed between two dates I could see myself wanting to answer this place between question in a lot of places and we happen to have a great new
00:15:27.240
place to hang that behavior now which is date range Cory Hayes has a cute name for this which is called uh he calls these Behavior magnets which I think is
00:15:34.519
really nice this date range class starts off very simple but it's a behavior magnet so what can we do now let's how
00:15:40.880
can we use that so in order in place between rather than doing this logic ourselves let's just ask the date range
00:15:47.720
do you include my place dat and
00:15:54.319
then uh we ask include is our question
00:15:59.399
uh date and we'll say is date after the start date and is
00:16:05.560
it before the end date we're still
00:16:13.240
green so I'm really liking this now this this this class this report this situation I'm really liking
00:16:19.240
it uh we have I like that when you order wants to know if something's placed between it asks the date range and all
00:16:25.839
that order really knows in this method is that the that you should pass in is the place dat attribute I like that
00:16:32.240
order knows how to answer that question I like that the behavior or the the calculation for determining if things are included in the date range lives in
00:16:38.920
its own class called date range that seems like a good place to to have it in there but Let's do let's do a little quick refactoring here so someone asked
00:16:44.519
if we could just use a range here yeah we we could use a range if I made this this could subass from range it could
00:16:49.920
compose a range um I'm just going to build the range in here on the fly so let's do start date end date. include
00:16:59.240
date and this should work the same way and it does one other thing by the way
00:17:04.319
that was pointed out in a talk and this is why it's fun to give talks to a room full of programmers that are pairing with you the ISS there's there's one
00:17:10.959
little slight performance thing here that we can do slightly better so if you ask a range if it includes something the
00:17:16.079
first thing the range does it instantiates everything inside it into a collection and then it searches through
00:17:22.439
that collection and sees if that thing it's looking for is inside there but since this is a range we really only
00:17:27.600
need to instantiate things on the end and then do some math and figure out if the thing we're looking for is inside it
00:17:34.039
and there actually is a method for this it's called
00:17:39.600
cover so just a tiny little performance win there maybe makes this a tiny bit clear what we're looking to
00:17:45.880
do uh a couple tiny little things to clean this class up and then then we're going to be done we'll move on to
00:17:51.320
another example uh for my public methods in particular I really like them to read
00:17:56.760
super nicely I like this for all things but particularly for these public methods that people are going to tend to read I want them to read very well uh so
00:18:03.039
the one thing I don't I'm not liking to hear it about this is this big medy junk uh pile of junk over here so let's
00:18:09.679
create a method uh called total sales that takes a collection of orders and let's move
00:18:16.640
this bad boy down here uh orders and we'll do
00:18:26.320
this see if we're still green and we are and inject this could be a little
00:18:32.080
bit simpler right we can just do this for feeling clabber uh we can also get rid of the
00:18:39.360
zero kind of why can't we get rid of the zero exactly the collection might be
00:18:50.120
empty right but the total sales of no orders is zero it's not nil or empty array or whatever we come back from that
00:18:56.480
probably nil right is not a number nil is not a number this man is correct I
00:19:02.559
wish I had a t-shirt to throw at you or something heavier
00:19:09.360
whatever so one thing to point out here I told you that parameter coupling is not great and that methods that take no
00:19:14.880
arguments are better than ones that take one and I actually introduced some parameter coupling here I'm passing in
00:19:20.360
these orders into this private method that feels a little smelly it feels a little questionable I incre I I think I've increased the readability I would
00:19:26.880
argue that the method called total sales within date range that reads total sales of orders within range is pretty darn readable but I've definitely increased
00:19:33.600
this parameter coupling here and so uh what was pointed out to me is that this probably means that orders the
00:19:40.799
collection itself wants to have these methods and that way I can just ask
00:19:45.880
orders. total sales with no parameter and orders. orders within range with no
00:19:50.960
parameters no parameter coupling I think it looks like these methods might want to move under the collection of orders
00:19:57.159
however that is going to be left as an exercise to the readers we're going to move on but first we've been working
00:20:03.200
hard uh so we deserve uh the one SC slide in my screen oh that didn't quite
00:20:08.840
work yeah this is the closest thing I have to a slide in my talk that's all just take a cleansing
00:20:15.840
breath as the cat dances by all right how how's everybody doing
00:20:22.559
you feeling good you feeling excited awesome all right
00:20:28.880
let's move on to another example uh all right let's pull up another example let me get my specs
00:20:34.880
going first and run them they're green okay here's some more code for you to read take a second and read
00:20:41.480
it this F could be bigger couldn't
00:20:52.840
it one more method down here that's the class
00:20:58.840
contact is looks like this
00:21:04.600
okay so what is not so great about this class well here's a problem so here's
00:21:10.960
the thing this represents a job site now every job site has a location jobs always happen somewhere every job site
00:21:17.240
does not necessarily have a contact we don't always know who we should get in touch with for the
00:21:22.880
job so because of this sometimes contact is going to be nil and because content could be nil we
00:21:30.480
have to handle that so we need to check for its presence and provide a default
00:21:35.520
same thing for phone same thing for email
00:21:41.279
contact this man has seen the future so what are we doing here we've
00:21:47.919
co-opted nil we're asking nil to stand in for a contact that doesn't really
00:21:54.120
exist because that's what we pass in here if we don't assign at contact it's going to be nil and so we're saying hey
00:21:59.919
Neil you just you just mean no contact and because we've co-opted nil and we're making it stand for something that it
00:22:05.520
really doesn't want to stand for we have to check for it all over the place so how do we fix this well let's make
00:22:12.080
something that stands in for having no contact so let's create a null contact
00:22:19.039
and so why don't we do that uh where's the best place to do this
00:22:25.919
guy line eight so if we do it in line 8 we're going to break a bunch of things we're going to have to refactor three
00:22:31.720
methods at once and we'll be red the whole time is there a place we can do it where it's smaller we can make a smaller
00:22:37.200
change and get green faster H and six I don't think
00:22:51.039
uh what about what if we did this
00:22:58.640
that work so this is going to blow up because null contact doesn't exist let's go to find null
00:23:07.440
contact and we need to give it let's not take too many steps so n contact now
00:23:13.159
exists but it doesn't have a method called name so let's give it a name and we saw before that the default should be
00:23:19.360
no name back to green this is something our CTO Joe Ferris pointed out to me so the
00:23:24.760
first I I did it the way that someone suggested earlier the first thing I did was say all right let's make this little contact up here but then you do you're
00:23:30.960
doing way more work while you are not green you're red for longer and it's actually worth thinking about doing
00:23:36.760
refactorings in smaller steps it's easier to keep it in your head it's easier to understand hi it's
00:23:44.120
okay this is Mr and I'll contact everybody okay so let's replace another instance let's do this here new. phone
00:23:53.120
this blows up because phone is not defined we'll Define it
00:23:59.520
back to green baby steps anybody feeling stressed of course you're not because you're taking Tiny Steps refactoring is
00:24:05.520
easy we could do this all day and here well I don't really see a good way of doing this here so now I'm
00:24:12.120
actually going to swap it in up in the uh the initializer n contact.
00:24:19.000
new this is saying now we don't have the deliver personalized email method on null
00:24:24.640
contact which is true this takes an email
00:24:29.760
and notice I can just leave this method body blank because when there's no contact we just don't do anything so I'm just going to Define that empty method
00:24:35.679
on null contact now we're still green but we've got this null contact line up here now
00:24:40.919
we get to do something which is every programmer's favorite job which is what absolutely there's no such thing as
00:24:48.039
an audience of programmers that does not know the answer to that question I've given this talk a handful of times it always comes back instantly everyone
00:24:53.679
knows what we're going to say okay so let's do this oops uh
00:24:59.440
um oh God uh
00:25:05.559
whoops does anybody ever have their Vim stop undoing am not the only one undo
00:25:11.000
does not work right now if I change this and hit undo it was like no go to hell let's let's quit vim and come back
00:25:18.240
real quick I need undo okay contact do name run the test oh jeez oh yeah we
00:25:26.120
need no worries Don't Panic M
00:25:31.480
spe okay that's the eror we want don't panic
00:25:37.600
guys all right good now more deleting
00:25:43.440
code and
00:25:49.000
this what's the my test say I'm okay all right so what just happened
00:25:57.480
well we stopped co-opting nil we made a very simple little class does anyone
00:26:03.399
have trouble understanding the N contact class no of course not it's insanely simple it's insanely straightforward and in exchange for that very simple class
00:26:09.480
with very simple methods I got to improve the heck out of this we deleted 10 lines of code we removed a bunch of
00:26:17.000
conditionals which is great because conditionals must die and also we removed the obsc the
00:26:24.399
obscuration the obfuscation of those methods check for contact all the time made it harder to see what those methods
00:26:30.919
really did I think this is a lot clearer in exchange for a very small very easy
00:26:36.679
to understand class now here's a question for you how many people have in their rails app right now a line that
00:26:42.559
reads something like if current user yada yada yada yeah put your hands up you
00:26:49.679
Liars okay can you see how this might map to that rather than having current
00:26:55.600
user return nil which then you're co-opting to stand in for not having someone signed in you
00:27:02.880
could return in you could return a site visitor object or something along those
00:27:08.159
lines and have a real class stand for that concept instead and clean up those
00:27:13.640
conditionals no one caveat uh people often ask how do you make this work in the view well it's more of a pain so
00:27:19.960
this is sort of a this we're sort of talking about following tell don't ask we don't want to ask contact if it exists we'd rather just send it a
00:27:25.640
message tell don't ask gets a little bit hairy in the view because a lot of times you're doing a lot of asking in the view
00:27:31.080
right A View basically exists to show you representation of something so I'm I'm less stringent about following tell
00:27:37.679
don't ask when I'm in the view but outside that I'd much rather just send messages to objects and not have to
00:27:43.559
worry and check for nail all right let's move along to one
00:27:49.240
more example and then where I'm going to summarize and we're going to talk about some other stuff and it's going to be awesome all right
00:27:56.279
so take take a look at some more
00:28:07.559
code so this is uh this is a user class and the user class happens to know how
00:28:13.600
to charge for a subscription it looks up a brain Tree ID based on some semantics and then charges
00:28:19.480
brain Tree by the way if you don't know is a payment processor there's a gem that uh handles that wraps up the
00:28:24.600
payment processing here's a refund that uses slightly different semantics to look up the transaction ID and then
00:28:30.159
refunds it now how is this code well it's okay
00:28:36.440
so most programmers are familiar with a key ID a key idea for easing change and
00:28:42.399
that idea is depend upon abstractions right like no one is shoving raw bites
00:28:48.039
out of socket right you're using a library that handles it for you most people aren't writing a ton of raw SQL
00:28:55.440
using something that generates SQL for you you're depending upon abstractions so that if implementation details change
00:29:01.159
you don't need to worry about it that makes it easier for your code to change they're shielded from changes at the lower levels your code doesn't have to
00:29:07.640
care as much so most programmers are familiar with that idea but what they don't always realize is that this idea
00:29:13.240
is sort of fractal it zooms out and it zooms in so just like I'm not going to
00:29:19.200
be pumping bites out down a socket I also don't want to depend on concrete
00:29:26.519
details in my uh business objects so when we have this yes I'm using a brry gem here and
00:29:34.720
this brry gem is an abstraction over making raw HTTP calls to brain tree but
00:29:40.000
it doesn't mean that these classes are depending upon abstraction for instance let's say I
00:29:46.000
change our payment Gateway now we want to switch to stripe it's a new hotness it's great we're going to try that what
00:29:51.559
happens well I have to go change my user Model A feels weird right I also have to
00:29:59.440
go change my refund model I want to change this one thing and I have to change it in many places one name for
00:30:05.480
this is shotgun surgery cute right another thing is this is Divergent change there are two
00:30:12.440
reasons for my user class to change one is that the business logic surrounding my user changes and the other is that my
00:30:19.480
payment Gateway changes and Divergent change is a sign that you have multiple
00:30:24.880
concerns inside a class so I've basically let the brain tree details
00:30:31.320
leak into user so here's one way you could fix this I would create a payment Gateway
00:30:40.600
object it takes the brain tree Gateway and I've moved all those methods from
00:30:46.080
user and from refund into here now this class knows the semantics
00:30:52.519
of looking at brain Tre things and refunding brain Tre things and creating customers and and user just knows how to
00:30:59.480
call up the Gateway and tell it what it wants done it passes itself in it doesn't know about the
00:31:06.279
details now it's easy to change gems right if I want to use a different brain
00:31:11.880
tree gem I don't have to change user at all if I wanted to switch payment providers which happens I've done it I
00:31:19.679
don't have to change user I don't have to change refund all those changes happen in one place the payment Gateway
00:31:24.799
class which seems to make a lot of sense that's where I expect to go to make those changes not user Also let's say I wanted to have
00:31:32.360
different I wanted to do different things when brain Tre is down let's say if brain Tre is down I can't just stop
00:31:37.399
accepting orders I want to do something like take on some Risk by just saying yeah that's fine we'll take your order and we'll charge you for it
00:31:43.600
later I can do that here I can swap in a Gateway that behaves
00:31:49.080
differently and the code that calls the payment Gateway never knows I'm not going to let that concern leak out
00:31:55.519
across my system everything else is depending upon that abstraction and so I'm able to create a nice encapsulated
00:32:00.799
thing that handles all of that finally it's nice and easy to mock this right in
00:32:06.600
my previous example if I want to test this method I
00:32:12.840
have to mock out methods on brain tre's Gem and that feels kind of gross I don't really want to do that it's possible
00:32:19.000
they'll change the API around on me I don't really like reaching into other people's code and and messing with it
00:32:24.679
but I'm totally totally happy to mock my own method me and mock out that payment Gateway that feels fine I'm totally okay
00:32:31.440
with that this by the way is an example of the adapter pattern I'm creating an adapter for the rest of my code to use
00:32:38.559
that sits between the next level the lower level details Okay so we've done some some
00:32:46.120
good coverage of different refactoring ideas different tools you can use but the question one question is when do you
00:32:51.760
refactor when are these things worth doing the first time is all the time and what I mean by
00:32:59.200
all the time is I mean after every change you make the cycle is red green
00:33:04.399
refactor do a refactoring right after the test goes green and then also when you check in get a diff of your changes
00:33:11.360
and look at them give yourself a mini code review see if there's anything you've introduced if there's any smells
00:33:17.279
if there's any obvious problems and fix those early always be refactoring a little bit and you don't need to go on
00:33:22.799
these like missions if you've ever had to had to convince uh a management of like look we need the next week to
00:33:29.320
refactor this thing so that we can do the other thing that is not a fun conversation to have no one likes to
00:33:34.720
have that conversation no one likes to hear that conversation if you're refactoring continuously you make it less likely that you're going to get
00:33:39.919
yourself in that state so that's the first time to refactor another time to refactor is when you have God
00:33:48.960
objects so what's a god object well a god object is something an object in a system that everything seems to rely on
00:33:56.639
everything somehow seems to connect to this thing and call methods on it uh there's in rails apps there are very
00:34:03.320
commonly two God objects the first one is what
00:34:08.720
user yes user and the other one is whatever that application is
00:34:15.200
about so if it's an e-commerce application it's order if it's a to-do
00:34:21.359
list application is to-do those are two likely candidates so when you are dealing with God object it's a great
00:34:27.919
idea to be extra aggressive with your refactorings and make sure you don't make them worse when you have to go in
00:34:33.599
there and touch user Make user better don't make it longer by the way here's a great way to get a feel for what might
00:34:39.879
be your God objects so I'm in uh an app models directory of an anonymous application that I will not name for you
00:34:46.520
I'm going to get the word count the line count of every class in this directory
00:34:52.000
and then I'm going to sort it can you guess what kind of application this is
00:34:59.640
it's an it's an e-commerce application so this is not a great
00:35:07.560
tool Pure Line count is not a perfect measure of complexity right if you have a class that has a very low line count
00:35:13.920
but every line is insanely complicated that's not great right but it's a gives you a rough idea what's a better way
00:35:20.160
well there's a couple things there are some tools like fog and fle and things like that I've been playing around with
00:35:26.320
this lately code climate it gives you metrics on your Ruby code and and it can even tell you every time you push if
00:35:32.680
you've made your code better or if you've made it worse that's a nice place to look to get you give you an idea this
00:35:38.640
is code climate.com give you an idea if your code is getting better or worse and those metrics are better they're better than a Pure Line count they're
00:35:45.119
considering the complexity the cyclomatic complexity of code do you have a lot of conditionals do you have confusing things do you have duplication
00:35:51.680
it's giving you a better picture of that so okay got objects all the time God
00:35:58.280
objects and where else High turn files so if you write a
00:36:03.480
horrendous piece of garbage and you commit it and no one ever has to change it again if a tree falls in the
00:36:10.480
forest right if you never need to read it and you never need to change it and no one ever looks at it well why would
00:36:17.200
you refactor that that's a waste of your time however if you have a file that
00:36:23.280
changes all the time and it's that same pile of crap that's definitely worth refactoring because you're in there all
00:36:29.160
the time and the fact that it's changing often is probably a hint that you don't have it quite right and it probably
00:36:35.200
warrants refactoring finally I just got
00:36:45.000
vimed finally a great place to look for refactoring is where you find bugs because what is a bug really mean a
00:36:51.800
bug means you didn't understand the code you wrote and doing a a good set of
00:36:57.400
refactorings can make that code clear the thing about bug is that they love company if you find a bug right here
00:37:03.280
they're probably a bug right here you didn't understand the code and you can make it clear by doing some smart
00:37:08.920
refactoring so before we wrap up I want to show you a couple book recommendations number one I think this
00:37:16.400
is just about the best General book about writing better code there is it's by Bob Martin Uncle Bob this is
00:37:25.119
excellent this is another one growing object-oriented software Guided by tests
00:37:31.560
I wrote this book off because the title sounded introductory and in a way it kind of is
00:37:37.440
but it's also not it's a great book it had it does an amazing job of describing certain things about tdd and
00:37:43.119
object-oriented programming in ways that I had never seen before and suddenly made me like see the light in like a
00:37:48.480
light cing down from the heavens kind of way really good writing totally recommended finally if you're feeling
00:37:55.960
the mood to learn learn if this has gotten you fired up learn. thot.com we get a lot of screencasts we got ebooks
00:38:01.839
we got blog posts we got a lot of stuff going on here there's a refractor screencast that you might want to check
00:38:07.280
out by the way there's we have a discount code for you Ruby comp 2012 all lowercase that's good for the conference
00:38:14.440
and then we're going to turn it off on you and finally anybody here listening to the podcast by any chance awesome
00:38:21.800
cool so we have we have a podcast it's often very technical sometimes it's about its entrepreneurs but a lot of
00:38:26.839
times it gets technical we talk about objectoriented stuff and we talk about refactoring and uh a lot of people have found it useful so you might too I think
00:38:34.280
that's all I have for you so let's take some questions yeah another recation is Michael
00:38:39.520
feathers leg the title as well it's a fantastic book on how to refractors
00:38:46.280
awesome that's not a question there's always one so the book was working effectively with Legacy code
00:38:52.200
by Michael feathers yeah yeah so you said you talk about God objects but could you give us any ideas about
00:38:58.240
suggestions for what to do with God objects yes extract Behavior out of
00:39:03.680
them yeah so God objects will typically have multiple concerns multiple responsibilities so find a
00:39:09.720
responsibility find a package of methods that seem to go with each other and represent one responsibility and extract a class with that that's my general tool
00:39:17.160
I think that's also do are you famili with solid are you fam with the solid principles it's a really good idea to follow solid principles in classes that
00:39:23.880
are God objects yeah uh for those of us not familiar with solid principles yes what
00:39:29.760
are they uh let me Google that for you yeah so it's a it's a It's s o l i d
00:39:37.440
it's five principles put together by Bob Martin Bob Martin uh one of them is a single responsibility principle and
00:39:43.920
there are some other ones they're basically guidelines I remember the others I just
00:39:51.440
don't want to waste your time I do we just talked about this no seriously no but take a look at those
00:39:57.720
are they're worth looking at and actually I think I'm pretty sure he covers a lot of them in clean code so it's worth taking a look for that yes hi
00:40:05.119
hi um so I I was looking at in your order class where you're passing in the
00:40:10.599
uh was it the customer and or the first the first example you had you passed in
00:40:16.160
a a contact name that might be nil yes and I I so basically the only way that
00:40:21.640
would happens if someone passed in a nil argument I you're forcing them but
00:40:27.319
you're forcing them to pass something in in the first place because you're because you're not
00:40:33.160
setting a default for it right but even if you made a default parameter someone might still pass in a nil object yes
00:40:41.800
right um but it would still be better to at least set it in the default right it would be better so because then they
00:40:48.119
don't have to pass it the M object to get a are you suggesting having like a default of a null contact for contact
00:40:55.160
yeah that's a totally reasonable idea someone still might pass well they're
00:41:01.280
jerks no seriously uh well what do you want me to
00:41:06.359
do when they pass a nil like raise raise you're a jerk don't pass nil yeah this
00:41:11.960
guy likes that yeah but yeah um I sometimes will put things like a a null
00:41:17.560
object as a as a default for a parameter but that's sort of a sneaky conditional right that's really saying if parameter
00:41:23.200
then parameter else null contact and like that just sort of adds a a little bit overhead for me so I'll usually do
00:41:28.520
that inside the initializer instead of the argument list that's depends on where you're feeling
00:41:33.640
confident like about your code is where do you want to fix it sure
00:41:39.280
yep yeah so at what point do you start adding tests for an object like null oh
00:41:44.760
good question so the question was at what point do I start adding tests for an object like the null contact UM and
00:41:50.880
it depends so sometimes I'll those little so that's sort of a little service class almost in in that particular class I'm using only in one
00:41:57.319
place right it's only involved in the uh whatever that class was so for classes like that especially if I make them
00:42:03.240
private I won't write test for it I'll test them implicitly uh or indirectly through the interface of the class that
00:42:08.880
does use it as soon as I pull that out and put it somewhere that other people are going to use it I'll write test for
00:42:14.119
that question yes um sort of follow on that would you didn't put it if it's a
00:42:19.800
one of the same file and you start using M into file or um yes so the question
00:42:25.680
was where does a go basically so if it's private I keep it in the same file uh if if it becomes a a an honest class in its
00:42:31.640
own a first first class class then I would move it out into its own file y
00:42:43.000
yeah uh yeah would I go a step further inject my payment Gateway into user Pro
00:42:48.960
possibly probably it would depend uh it it would depend on if there seem to be a
00:42:54.359
good reason to do that wouldn't it make make user more of a god object shouldn't be separate functionality wouldn't that
00:43:00.079
make user more of a god object no I don't think so because it's already using the payment Gateway um so he's asking I could just inject that payment
00:43:06.800
Gateway instead and yeah that's that's totally reasonable I could invert control on user and just pass that in there yep yeah
00:43:17.040
John yeah he's saying if if different users could have different payment gateways and I would definitely inject it but and that's that's only one
00:43:22.359
argument for dependency injection though it's also nice for testing right for throwing stub in there or some sort of
00:43:27.760
mock object instead of an actual payment Gateway yeah yeah can you talk quickly
00:43:33.400
about The Decorator and also use
00:43:39.720
that uh so this guy is holding me to the the talk description that I wrote uh six
00:43:44.839
months ago uh no I'm not going to talk about The Decorator pattern but I will talk to you afterwards if you want about it um
00:43:50.599
and then a gem uh some people like Draper uh we haven't found a great use for it yet
00:43:56.680
when we want to do a decorator we generally do it we just sort of roll our own on the Fly what's that your death is my
00:44:04.960
decorator I like that that's a good t-shirt yeah what do you think
00:44:11.160
about uh actually we just recorded a podcast about like in the DCI context is that you're thinking so we recorded a
00:44:17.359
podcast with Jim gay the author of clean Ruby and we talked about that at length and so I encourage you to check out that podcast when it comes
00:44:23.599
out I can't duplicate I got to stay dry
00:44:39.160
take the path that you did when in the example not
00:44:49.599
prior whereas lot of the see we're going to kill
00:44:54.960
that
00:45:00.760
so his rough question was why did I take why did I prioritize taking small steps when we did that null customer
00:45:05.880
refactoring and so I sent a copy of a recording of my talk to our CTO Joe Ferris who is right there and one of his
00:45:12.520
points of feedback he he actually pointed out two times in my talk he said you can refactor in smaller steps here
00:45:18.880
and I thought it was really interesting that he had picked up on that because that's something that I don't always think about is but it's actually a
00:45:23.920
really good habit I think is taking those little steps right you want your test to be as green as often as possible
00:45:30.079
because whenever they're green you know you're good you know things are working and the longer you go in a cycle when things are red the more likely it is you
00:45:36.440
can kind of go off the track right is that a fair description Joe of the advantage yep Joe says yes thumbs up
00:45:43.559
proved by Joe so that's that's so that's so that's a great question because it was something that I didn't think about a
00:45:49.440
lot I'm think I'm more cognizant of it now and that's something a habit that I want to improve
00:45:54.640
on yeah um use objects in the cont of
00:46:03.359
associations yeah so he's asking if you can use null object in the context of belong to associations um as long as you
00:46:08.720
are in a recentish version of rails I believe you can get away with this in a sane way I think if you're back in the
00:46:14.400
two something days it's it wouldn't work but I think in three after 32 is anybody
00:46:20.200
know for sure you can call Super and then handle the using a null object there so yeah so give it a shot I think
00:46:26.800
it works anybody know it doesn't work all right must