James is a reformed topologist, now vowing to use his skills for good. He is currently an engineer at Procore in sunny Carpinteria, where he enjoys refactoring and long walks on the beach. In those rare instances when he's not coding or evangelizing category theory, he also enjoys drumming - mostly math rock, natch.View the profile
About the talk
RailsConf 2019 - Refactoring Live: Primitive Obsession by James Dabbs
Let's roll up our sleeves and clean up some smelly code. In this session, we'll dig in to Primitive Obsession - what happens when our domain logic is all wrapped up in primitive data types? And most importantly, how do we untangle it?
I'd like to thank and control help me think that the sponsors for making this possible in the organizers and staff were actually making this whole thing happen real quick Round of Applause. Thanks for all y'all. Hey everybody. I'm I'm James. I'm here today to do some coding with you and came to me by way of the clicker. That works. Good computer while came to me by way of. These people who wrote this book and these people who wrote this book that most of the concepts that were talking about today or not entirely new to you.
If you have maybe tried to use some of these and I have really permitted your day today work. If you haven't had the time to practice or had trouble putting them into practice in a rails application of scale this talk is for you. How many people here have seen this talk good with us from Rosemount 16. If you're watching this on video at home or otherwise have control of time. Feel free to pause me go watch that and come back in like a minute and a half to do a quick recap for anyone who's stuck in the here and now you're the
definition. It's a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable Behavior. SoCal important things there. It is purposeful, right? We are making a change to make something easier and it does not change their behavior. Right? I'm not altering any of the contracts and user experience the process of doing every Factor. That's the idea of the big question is. Okay. How do you how do you actually do it? And here is the hardest part of the whole thing if you have coded in production
if your customers are happy if your servers are happy if your air monitoring services are happy do not touch that code there is really nothing you can do to improve it and there's a whole lot you can do to make up for what you need to do is wait and that's super duper hard. But wait until you have a new requirement. Once you have a new requirement, you can ask this question is your code open for that new requirement. They open that means something very specific. It doesn't really mean do I see how I can add this and I think that I want to do any bugs. It means 10. I add this new feature by
only adding lines when not changing any existing definitions or methods in my coat at all. Right is it feels really open? If it is. Fantastic here. It's easy making any change move on to the next thing I had similarly if it isn't but you see how to make it open. That's fantastic. You're done. The interesting part is what happens when you need to make a change, but your code resists that changed somehow and in a way that makes it not obvious how to proceed in that case 3 factoring books say what you do is you pick the code
smell closest to your new requirement. And this doesn't mean some big hotels very specific thing. It's not just something you don't like about your code will talk about when the second Don't you want to go smile? You apply a Curative recipe for that goes now, I want to keep things about coat smells is that each one comes with a handful of Purity recipes have some process that you can apply to fix it. So you do this over and over it eventually or cut it open you make the change and you're happy.
Today we're going to talk about a particular codes now called primitive Obsession and permitting session is the tendency to use primitive types to represent. Your business objects is a really easy things to do in rails near very often. You'll do something like write a user model that has a phone number that's a string by string. But the Temptation is too then think of the phone number is being the same as a string and when you do that you end up writing code that looks like this what you really need to express is that you owe your phone number is local Right light
Concepts in your problem domain. So we'll look at some examples of the question is when you find that you have this problem. What do you do a nice thing is they're a handful of rest before to look at two in particular. Look at replacement with object which has this picture in Chandler talking book. It's got some steps their broadly make a class. Make sure you didn't make any send text errors use the class. Make sure the test pass and a more complicated one where to look at replace conditional with polymorphism which looks like this. Good luck.
But here's kind of what it does to code. If you can see this you start with some complicated condition with lots of branches and you end up with several different classes with simple winter faces and relatively simpler methods and it has some steps right create classes for your behavior creative Factory function to pick the right class. Gotta gotta gotta the point is not to memorize any that's right. There are 20 some recipes familiarize yourself with them and then go look him up when you need them to give you a feel for what they look like exactly but really like don't
bother trying to Commit This to Memory where to find the missing a real actual rails application. So we are going to be doing some live coding. Everybody ready? Here we go. Okay. So this is a sorry. It's a it's a it's an app for managing College records braids enrollment and stuff a few of the main concept but have to stand by the laptop subdomain Concepts a section is kind of Korra Korra I've ever even talked about a section is a particular class right like math 101 running in some term fall 2019 and some room
with instructor students are enrolled in the class and the enrollment is that joined record where we record the students grade in that class you have assistance is is that you're here. When the core responsibilities of section is to decide whether or not it should admit a particular student and it does some kind of straightforward stuff with checking role and everything but the interesting bit is it verifies that the student has gotten grades and all the prerequisites for the course and that the grades meet the minimum
requirements Uma Thurman Timber Court participant on the right when I got running is guard set up with R-Spec. So what you'll see as I change files and go save the second file over on the right that's going to run the unit test for the section model and then assuming they pass it around the full spectrum of that unless it turns red. So if it turns red will like this front part of the room, please yell at me very loudly and make sure we stop and rest it. But otherwise we're like the other ones were not going to worry about it all encourage you to look away.
Okay. So one of the other things is that does is we have transcripts a transcript is largely responsible for computing a student's GPA. It does that by pulling their enrollment grades looking them up by the appropriate chores for them and doing some of the shop is not terribly interesting or important right here right now works fine, but it only understands whole letter A B C and D. So that's fine. But let's say we have any requirement we would like our app to support plus minus grades.
What will work a little bit on defining some acceptance criteria, but let's say when we do that where we end with with a couple specs this one in the transcripts that probably says if I have a student to that'll be pluses. I can computer GPA appropriately this one fails for not interested reason. So I'm going to skip it for now. You'll see it running as as pending in the stacks. And then in the sections back this one, that's somewhat more interesting. Somersaults on this for just a second. Bizpac says if I have a section that requires you to pass a prereq with a c
and a student who has passed that prereq for the C. Plus they should be admitted into the course right reasonable requirement. This does fail. Nobody help me. Please give me honest. It does fail, but for sort of interesting reasons and fails because I have a validation on enrollment that says that that's not a reasonable grade. I'm going to cheat just a smidge and add these just we get to the the the interesting failure. Nothing will fail don't yell this time? Sorry, if I say is going to fail don't you know, you got it.
So we would like to this path, but it doesn't and it's for a certain age rating reason. Let's dig into the section implementation just a little bit. The issue here is it when I get to the point of checking my requirements. My minimum grade is a c like you would expect the great. This one has a C+ which means they should be admitted that Klein. That's why you have one number 743 and check out the the line right below that the problem here is that when I compare grades I ask is the grey that I have before the grey
that I have to have that works pretty well because a comes before c and b comes before see the problem is C plus does not come before c. Any more than aardwolf comes before a right like these are strings and if you take a look at the enrollment grades that we have. Right. I have written these down into the ascending or descending grade order. But if I ask them I got something very different back. The core problem is that I have treated my grade test ratings
and that was fine to a point. But now I'm understanding with the behavior of a string is different from the behavior of a grade so I have to untangle before I can move forward with this requirement. That's it. I'm not going to try to actually actually six requirement for a bit yet. So I'm going to comment this back back out I expected sale and I'm just going to work on refactoring towards making it open. Thank you. Okay, so now I understand that. I have a problem session. I turn towards my recipes and I'm going to apply extract object for this value.
The recipe says what I want to end up with is it I want to end up with an object like a new class. It just has a field that raps the grade the code that I want to see you look something like this. I'll wrap the grade. I'll ask first name back right super minimal change. What season of ambulant I mean new grade class. I'm going to initialize it with a name which I store and I want to have a reader for that name. That's it. But with that implemented I can change these lines and it should work exactly to do before. That's it. We've been
refactoring that is the entire thing right? And it doesn't feel like very much because it's not but the nice thing is I've made a small but measurable step and I've accumulated basically no rest along the way. I'm going to take a several hundred more of these and by the end, I hope to have something really happy with but that I've never was scared about the entire time. So this isn't much of a win, but it does give me a place to start looking for taking some Behavior off an object that I own but doesn't ring. so I'm going to carry on and extract that comparison out to an object on the grade
class again the code that I want to end up with like this left hand is hugely confusing and hurts my brain to think about I want to end up with some code that looks like bright is my grade at least this minimum grade. And that's super he's at right I own the grade class. So if I have some other grade, I'm going to resist the temptation to fix the problem. And it said I'm just going to move the logic into place. The ones that same place I can translate that line and inspection still pass. I got to go to hire these on the way. They're not interesting most the
time. I'm really focused on the code changed here. And it starts to feel like a win right? I am now comparing grades as grade objects and I could go through my system and everywhere. I make a grade comparison. I can replace it and I can go to do that. It's not terribly hard to do. I just can I have one other call site so I can make a similar translation? What was that? I couldn't I couldn't do this one of them throw the app, but the problem is I'm still treating a greatest Rings everywhere
throughout the course of the app. And this is the boundary is how do you compare them? I'm converting an object would really like to do is make my system always talked about the objects in my system. Right? Like I want all of me like every time I want Brittany grade and then if I've been lots of grades on comp in my system works, so I want to start moving in that direction. Let's take this as a particular example. I'd like to end up in a world where user. Grades returns grade objects. I'm not comfortable making that change though because I don't know what all calls user.
Graves yet. So first I want to insulate myself in that change a little bit and I can do that by adding this. So currently raises hash I'm going to convert all the values to grades now this will fail Can I tell me why? So that the tricky bit here is I have been converted to Grace twice. Right initially. I transfer values then when I pulled it out of the house I passed into her braids again. So this is not a safe. I need to revert back to back out. So I'm going to head that way what I need to be able to do
this cuz I need a method of taking a value that might be a string and might be a grade and ending up with a grave. Different ways to do this, but one that I particularly like is at a factory. So when given the value I can turn the value if the value is a grade. And otherwise, I'll assume that the string and and make a new grape nut value. Laporte exactly, like new except when Hannah degrade. I just got the same object back. with that in place I can update my calls to new with for continent that if something Upstream changes to pass a grade down instead everything should
continue to work as it did before. And then I'll make that change Upstream. I'm back. I'm going to try to use for consistently instead of new so that later when user changes up from that. I'm also insulative that change right just kind of adding a shim layer so you can be permissive of what you accept an option code. Once this is in place, and I know that the grades received from here are great objects. I no longer need these updates. And I can start simplifying this this block a lot.
All of the changes are safe. Can you do the same thing the schedule the exact same Concepts right? Instead of New England a new gray. Make every time I just want to convert. Then I want to take the Upstream grades. In this case that comes from a method on self and transform them. And finally, I don't think I need. That indirect anymore. I can move there. Now there's nothing has happened at this point. If I take a look, I sort of made a couple of the coal sites to grades safe. Let's take a look.
If you notice there are only three coal sites praising my application to that. I have immediately cast two objects after calling them and one in specs. So at this point, I'm pretty confident that my app wouldn't change if I change the behavior that method but I should think about what the actual test Cadence has right like the specs say that I get string like object back. It's totally reasonable here Chase effects to reflect the change that I want the code, but my grades are spring like objects at least back has written should still pass. I can do that by
defining equals method. and saying if the thing that I'm comparing to is a grade Can I check my name against the other grades name an otherwise just you can have the default thing. Something with that change the spection pass and all of my call sites are safe whether or not I return braids are strings. And I think at that point I am comfortable making the change to the user grades method. The only returning great objects. And all the sections will pass. I want that done. I
no longer need the shim layers. I don't really like I've gotten somewhere if you look at our implementation of section as far as section understands, the world grades are grade objects. It's not you because throughout the app, but I have layer upon layer pulled back and got to push the strings out towards the boundaries. Singing backwards requirement right and let my grades me understand string if you think a little bit about the application right now, we're still we're still treating a great as a string in the place where we can keep score in the transcript.
So we'll want to do a similar treatment here, but I think we need to know what I want as I want to be able to write not take the string braids look it up in a hash. I want to be able to get a grade and ask if Earth's core. Now won't quite be able to do that. Right enrollment is an active record object created a string call him, but I can at least Can you convert that ear for now? And it feels like I should be able to write that. So let's take that as a goal. Right to do this great object me to understand their score. Right now knowledge of scores is wrapped
up in the transcript, which is obviously in retrospect the wrong place for it like this. This prefix is a clue the Great's worst kind of belonging on grades. Going to change name to scores. But with those here, it's easy enough to write the store method. the score of the grade is welchol except Now I don't love that. It's possible to create a grade that when you ask it for swore to blow up and I want to fix that, but it's not required yet. I'm just trying to get this line. starting transcript this line State
grades have a square method. So this should be safe to change. I will do that. infection to pass field the Chelsea at Willows Unified insurance Braves score call the doors. I looked up in store. So that wind is not safe as well. And I'm not referencing grade scores anywhere so I can lick that as well. Show me some really good progress. But I think we're at kind of the hardest point. And and also when the key points here if we take a look across the application at the places that we are converting a string to a grade it happening in two places here in the transfer if we have an enrollment reckon
an active record object that's giving us a string and similarly in grades. I pull these are history and it's it's still enrollment record that still treating greatest strength. If you look at that actor record object we noticed that enrollment has its own separate notion praise and comparing enrollment and grades together. There's a lightning bug here right enrollments can record the grave with an F on my transcript have no idea how to handle us whatsoever. Right? The fact is loaded with written in multiple places his invited a bug. I'm going to call that a bug and just add this
although not prompted by specific requirement. But any case like we need to unify enrollment with the rest of the system, this one's harder though because we don't have a grades method to shim. It's provided to us by a cube record going to have to do something a little fancier him. But but similarly what I want is not like I don't want enrollment have its own notion of grades. I want to write something that says What's the grade for this record is the great for me to treat that field as an object in my domain? I'm going to do something special but active
record supports this I just have to tell it to serialize the grade field as a great object visible. I'm coming. Out for now is a couple pieces of going to have to implement to make this work one will need a great. All method, but that is easy enough that. Right. I've got my list of stories I know about was just map over it. These are immutable so I would never want us that although it's not strictly necessary. Secondly, if you're not familiar with serialized the contract that I can record expect of you is that the class
do you provide to serialize should respond to load will just rang and he produced not registering or kneel and dump which is similar, but we'll need to produce the value of cereal is a database. So we need to find those. What about you if we don't if the value until we return otherwise converted fourth grade? And dumped is exactly the same except we need the the sea realized value right there in the face and name of the grade. Do with those in place on Counting these two on should work. We have to think a little
bit. There's one change the plate in here. What's not clear? We are changing the contract of the grade method on this object. We have to think about are there other places that are calling grade. We saw that we send several of them, but I haven't necessarily audited the whole app to make sure that we have this is a point where it's really nice to have a good test we can meet on so I might run this and just the okay. Well what still blows up there? Otherwise can have to go line-by-line and and find every color. And this case we should have just a couple.
So these are coming from the grades controller which used to return a string and now is returning name corresponding to a string. Probably not terribly surprising. Write in my grades controller. I used to return a grade which was a string and now I'm getting back on Spotify. Great object. We can easily add. Name here and that's really fair. But I think the better operation is to say anywhere that I'm serializing a great night. If you die, I don't want to change anything. I just want to produce Jason values or just a name similar value objects that represent primitive
types. It softened reasonable to define a conversion method back to the base type. I think with that change all of our tests should pass. So the nice thing here is at this point now everything in my system that talks about grades uses these objects. So if I want to make improvements how grades work. I have one pile to look at. And I can start exit. Someone changed I I would like to make all those not directly required. It seems on to me to passing the name and ignore the score. Like I really want
to think of grades as having a score. type a pin that is a change to the API for new so I need to make a default argument if I Want to Break Stuff. and one of the ways that I have found super helpful to get my self-confidence that I am free to make changes to that method is especially if I have a factory method is to make The new method protected write my expectation is that anytime he wants grade. They're calling for I can make it explicit by making new a protected method on the class. Anime that I can watch free or
two to change these invitations here. It's easy enough to create a great the score. I made a reading over the list of stores. Hear something slightly different needs to happen. Right if I'm if I'm asking you for a great outfit that represents be something has to know that Abby is a 3.0. But really I think I think that all should be the source of truth about what grades exist in my system and I think four should delegate also I would tend to want to write all find the particular braid. that you want or maybe raise a particular.
Hopefully helpful error message. And that my grades have a score. And so I can I can think about comparing them pretty easily why Ruby is a good tool for this if I want to compare two things. I include comfortable. I deaf spaceship operator. And now since grade objects have stores. I can easily express that the way you compared to. Another thing is you can pair. the scores of the two things that grades and they still have to rescue from an argument error if I'm given something that isn't grade like
right place. So if you're not feeling comfortable what that does is that gives you greater than greater than equal to equal to for free. I don't need my equal to meth anymore. And finally and most interesting Lee my at least method becomes a lot easier to write write instead of comparing names, which I know won't work with C plus has friends and I can read this. I want to look very similar. It is I can't actually very different thing instead of saying my name as a string is before some other string I'm saying me as a grade. I'm bigger than this other grade.
All right here. I can tend to you nothing. I can send the nothing about the way the system behaves has been updated at all. But now if you want to look at our transcripts back. You coming. Back in and our sections back and comes back in for those two paths. We need a B+ and a C+. So they are failing. But I can add a B+. Hennessy Plus and all the sexual past we refactored until it was open for requirement and actually implementing the requirement to a few lines. If you heard make things easy to make easy
change this I believe this is what that means. We get one more. Cool. Thank you. What's it look like green controller right now? When you're checking if you are allowed to change grades will use the same method in both places. If this can upgrade update braids method which structure roll in a couple days or not. And then you can if your teacher you can if you're the instructor for Section Niger student you cannot be ready soon is the implicit Circle. I'd like to introduce a new requirement pretty good
and I'm confident in them. So I'll just add this new spec. If you're a student assistant, you should be able to record new grades. You can't record your own grade though. You can't update other grades, right? So these fail I'll be done when these are passing. can do this It didn't have to copy and paste in create a can create grades method and then start changing conditionals. Right but my requirement is to add a new role to my system that has new behaviors. And if it were open I'd be able to do that by adding a new rules my system that has new
Behavior by the problem is I tried to represent rolls as strings. And so every time I need to make a decision about how they behave I have to write it right there cuz I don't have an object to hang that behavior off of metal. The way I'm going to fix this is by replacing these calls with polymorphism. I want to end up with something that looks like grab the roll for that user and ask it if they can update. So over in the section, I'll start to add that the again the the recipe here. It says Define a base class.
Play trolls. Define a factory method. That Returns the right kind of thing. Copy the method over the you want extract. Update it appropriately for its new surroundings. In this case. I want to ask can you update and I don't have current user I have user. And then wire that class in right so I want to write this on my second object all the find a role for a user. Which returns? That's a big change that I tend to like to run it once just wearing it in without changing return values maker didn't introduce any errors and then
running the whole thing. But with that done assuming aspects all pass and we can focus on the role itself yet. He is I want to move this conditional up and give myself different kinds of rolls. So I'll start by extracting an admin role. And the way and I'm enrolled works is at Men's can update. Right. That's that's a business requirement. To make that work. I need to make sure that the factory returns an admin role when appropriate. But doesn't behave any differently other cases.
Once that's true. I never had that brand so I can just throw in are there just to make sure for myself? I'm going to extract this new section user bit too. So I don't have to keep typing it. Right now I'm trying to pick the right type of object. The object should all behave the same. So that seems fair to me. And I can continue attracting but it's to finding other roles of other types. Write a teacher role. behave this way Justice logic You get a teacher roll? under these cases
and with that in place, I shouldn't hit this Branch ever. I probably won't like ball. The last World doesn't work. I think in my domain what's really going on there is there is a student roll. What can I ever update? That place I wouldn't implementing update method on the base class at all or would have raised some descriptive a her saying subclasses and implement this something like that. But that's that's my extracted hierarchy over the graves controller and inline these
methods. Right. This is how can I update grades currently behaves? I want to create a new. pre-check the problem is for creating braids as an assistant. I'll need to make it or not rating for myself. So I'll have to pass that in right this is it this is required change, but with this hierarchy it's easy enough to add if I want a create method everywhere. I can a database class by default the implementation it to check. Can I update all grains car wash, right? and with that in place
And without the context are there save we should be able to make a change right? Nothing on Behavior change. We just going to declare these like pulling out separately. I have not changed a bit about how the system to hips this point. But now if you're handed this and the new requirements, they are straight for it. Like I'm literally eating Implement right like either there's nothing to do but the right thing which is if you want assistant role will make one. Define how it works.
If you're an assistant, you can add a grade as long as you aren't the user you're adding a grade four. And specify the conditions under which that is your role in this case. It's if the section assistance include the user Will that change I believe the section pass? And again are our implementation of the future was. five lines of addition write 10 lines about a code didn't touch anything else, right, but we made it open and I made a change. That's it. We did it. We got through it. How is it going?
7 takeaways from working this way. I think moving towards developing in the store to mode make a coupling easy as it used to be really hard. You can get started without knowing what the code has to look like when you're done. It is literally a follow your nose for the thing you can like as long as our customer green. We could have stopped and ship production at any point in time. Right? When you going on vacation. Are you picking up some new task? Are you going to lunch doesn't matter? It's good to go rent. It may not have finished may not have gotten what you wanted to but really sweet
like this is software never done being comfortable with intermediate steps as being finished products as well as is important thing. There are some hard things that are still hard one of the key bits here would like a quick feedback for you. I would bring you want to run a small set of tests on small object that don't have to talk to Dad Ace refactoring will help you get there. Some other things that are hard-working clickers trusting your coverage right there were some points where it was really helpful to say like, oh, it's something broke. I'll see you know, silver
bullets are right, but also consider some in production testing, especially I found some science experiments be valuable or you can push up egg a tentative change production and get some notifications if it actually would have changed anything so you become a confident that you didn't break Behavior. If you're referring a lot, you'll want to push hundreds of of changes to production that don't change anything. And if you were asking a human who verifies your work to manually regression test every time you're both going to be miserable. Don't do that. If you have to work together to
develop a set of regression tests that you're both confident in also give them some insight into your development process, right leg refactoring is a process 4D risking the development process, but a lot of times you log code of all people won't see that isolate yourself Andreas man's where you can but don't be afraid to use it when necessary to record is the boundary between the database and your world that you are creating t-shirt about your world so that you don't have to worry about database things. Weirdly, none of this stuff gets harder
and larger apps except finding all of the places that you call a method. That's the one thing right? So if you want to make it easy on yourself use big long descriptive overly verbose names don't get fancy with Dynamic sending and consider maybe like making a new methods which in the college Overlook deprecating the old one instead of just deleting a rewriting immediately and place. But if I can leave you like one big take away from this, but how many people have expressed this sentiment like this? Right? Like we have some debt that we love to pay off but we have to ship the features
were too busy. Right? Like I said that plenty of times more and more I have come to realize that I think that is exactly the wrong perspective on the one hand. If you do not need to pay your product, there is no reason you change your code right? There is a reason why I got a new requirement but on the other hand if you need to change your code, why would you ever do that? Anyway, but doesn't make it easier to change right? It's not the real factoring is the thing that you could do after your product refactoring should be the thing that helps your product be easy to make a process. How to
take away the Greeley books are great practice practice practice if your company can bring in San Dimas for workshop with do that. It's amazing if you want to work at a company that I took a picture at work the other day there were Dolphins. We also having an awesome if that's your thing. Go go for Sandra tector. And that's what time but I'll be hanging out and having a chat afterwards.
Buy this talk
Access to all the recordings of the event
Buy this video
With ConferenceCast.tv, you get access to our library of the world's best conference talks.