 Welcome to So You Want to Start Refactoring. My name is Jillian Foley, and I'm going to be talking about refactoring. So just a sort of an opening question. Who here has spent a significant amount of time working in code written by other people? Okay, so most of you. Is there anyone in here who has spent most of their time working in only their own code? Okay. A few of you. So a little bit about me. I'm one of those people that has pretty much only worked in other people's code. I'm right now a software engineer at a company in D.C. called Contactually. I started coding on the job. I was sort of thrown into a programming internship by my dad, not knowing how to code. I feel bad for that boss. So basically this means that I've learned how to deal with other people's code before I've ever really learned how to write my own code from scratch. Which means that I've done a lot of refactoring before I even knew that word. I actually didn't really know the word refactoring meant. It seemed kind of scary until maybe two years ago, actually. I've been coding RAIL since, technically since 2007. So pretty much only worked in legacy code. And I'm sure that since most of you have worked in legacy code, do you know what I mean? But just to clarify. By legacy code I mean code written by other people, code that's been around a while. It's had multiple versions, it's been touched by a lot of people. Code sort of develops cruft over time. So this is pretty crufty code we're talking about. And then one little more bona fide about myself for this talk is that one time my boss gave me a box of pop tarts with my face on it as sort of an award for refactoring an internal tool. So if you too refactor, you too can get a box of real pop tarts with your face on them. And those are real pop tarts there, as you can see, cinnamon roll flavored. So what is refactoring? This is the million dollar question. Probably some of you know what it is, but probably some of you are at this talk because you're like, I don't know what that means. Here's a good definition I found. This is on Martin Fowler's website for his refactoring book. So refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior. So I like this definition. I think it's pretty precise without being too technical. But just to break it down a little bit, the idea is to not have any change to your users. Your app stays the same. They don't even know anything's happening. But you have an improvement for your developers. So you, your teammates, other people that work in your code see the code getting better even though your users necessarily don't know that anything's happening. So one other sort of quote that I think that I like for this idea of refactoring is from a sort of hoity-toity architect named, well, not named, known as Le Corbusier. I don't actually know what that means. It's we must refuse to afford even the slightest concession to what is to the mess we are in now. So in his little hoity-toity architecture world, he was talking about improving the future and building spaces that improve the future. But in our sense, this is sort of like if our code is a mess, it doesn't need to stay a mess. It doesn't need to be a mess forever and we can fix it. So why would you want to refactor? Obviously, I've mentioned one big reason is that your code is a mess and you want to fix it. But you'll see this question a lot, especially if you work on a bigger team or a more corporate team or a team that has a lot of external deadlines. You'll get this question a lot from people like product managers, CEOs. We don't have time for that. We need new features right now. We need new features all the time. We don't have time to waste time on cleaning up our old code messes. So sometimes it can be hard to get management to prioritize code practices. So I'm going to try to talk a little bit about why you'd want to refactor and what you can tell these people to even get them to agree to do it before we get into how exactly you do it. So why would you want to refactor? I see there's four big reasons here. We have maintainability. Those of you that have worked in other people's code know that it's always easier to work in other people's code when it's well organized. When you know what you're looking for, it's much easier to go find a bug that you've seen before. If it's code that you know where to find stuff, it's easier to add new features to code that's well organized. You're like, oh, this fits in exactly in this slot. Scalability. There's two things here. First is the usual scalability. Again, it's easier to make your code perform it when you know where things are, when you know you're not accidentally making the same database call on 47 different files. But it's also, refactoring helps make your team scalable. So this is the beginner's track. Some of you might be beginners. Some of you work with beginners. It's much easier for new people, whether they're new people to code or just new people to your team. It's much easier to onboard them onto your team when you don't have to sit and explain your code for three weeks when they start. I see some people nodding. You know what I'm talking about. Flexibility. This goes along with maintainability. It's easier to do those pivots that people in startups like to talk about so much. If you can see the code that you want to change, you don't have to go hunt it down before you start working on it. And spring cleaning. This is a good one. If you're talking to someone who just doesn't see the value in it, if it's important to spring clean your apartment, clean out your closet, take your car in for an oil change. You spend way more time in your code than you spend in your car. So why not put the same investment into your code that you would put into your car? So now that you've convinced your management that you think it's a good idea to refactor, they agree with you. They think it's a good time to refactor. How do you know that it's actually time right now to refactor? So who in here has come across code that they couldn't understand or that seemed poorly organized? Yeah, most of you. So if you're a beginner, sometimes you come across this code and you think, oh, I'm just a dummy if anyone was in the talk before this. That's one of those things. You come across something and you're like, oh, I'm just stupid. I'm too afraid to ask. I don't want to make a mistake. A lot of times the code really is pretty hard to understand. It's not just you. It actually is the code. And so I think this is something that's important to refactoring is to trust your gut and to know that if you think something looks weird, chances are it might be a little bit weird. So if you think in your soul, you really feel that it's time for refactor, you might be right. But here are some warning signs. If you're overwhelmed by bugs, especially bugs coming from a single part of the code, if you're seeing decreased performance, this can come from a lot of factors, but refactoring might help you pinpoint what some of those things are before they get too bad. If, again, a new person comes on your team and looks at this code and says, guys, what on earth is this? You might not have someone who's that bold, but sometimes you do. Who comes in and is like, this code is crazy. What have you guys been doing this whole time? These are all good signs that you should go into your code and take a look at it. It might be time to do some spring cleaning. In the Game of Thrones world, winter is coming. It's time to make sure that you get your harvest in before winter comes for, God knows how long. But sometimes you don't catch these things on time. And then you end up like this guy. So you don't want to end up like this guy. These are signs that you've waited too long. You need to refactor right now. You probably should have done it already. Tests are increasingly hard to write. I like to point this one out because I think that in a lot of cases, tests are sort of the canary in the coal mine. If writing tests for a bug fix or writing tests for a new feature is increasingly hard because you're relying on lots of outside assumptions or it requires a lot of extra pieces to set up and to stub out before you can even test something. Maybe it's time to think about how you've structured that piece of code. I'd like to point out here that tests can also be refactored just because your tests are hard to write. It doesn't necessarily mean that your code needs refactoring. It might mean that your tests need refactoring. I've also seen code that's so poorly organized it's blocking new progress. It just takes too long to write anything in this piece of code. So you can't add new features there. So that's a good sign that you should have gone back and refactored and now it's the time to do it so that you can, like I said, be more flexible, be more scalable, things like that. If your code is crashing, that's probably a good sign that something is wrong. Again, especially if it's coming from the same piece of code over and over again, production crashing might mean that, yeah, you have a bug, but you might have a bug that's colonized and is spread and the whole code just kind of needs to be cleaned out of all of these bugs. So the meat of the presentation here, how do you do it? There are lots of different approaches to refactoring. This is sort of, I'm just going to walk through how I generally approach it. There's lots of different ways. If you've never done it before, this will sort of hopefully give you some ideas of how to approach this concept. If you have done it before, maybe this will give you some new ideas. So the first thing I like to do is prepare. So I have this phrase up here, mise en place, which is a French phrase that people use in cooking. That's basically like, if you're going to start cooking, the best thing to do is to get all of your supplies set up before you even start. So measure out all of your ingredients, put them in little bowls, get all of your tools out, lay it all out, so that way when you're like stirring sauce on the stove and there's tomato sauce splattering everywhere and you realize you need the salt that's all the way across the kitchen, you don't have to like run across and have everything burn. Everything's terrible. So before you start to refactor, you want to do that kind of thing to make sure that you're not scrambling in the middle, washing stuff burn and your kitchen go up in flames. Some things that are important in this process are beefing up your test coverage. In the Rails community, we have a lot of cool tools to make sure that you have good test coverage. This is the time to get out those coverage reports. This is the time to look at those kind of code quality metrics that I'm sure a lot of you use, maybe 17 different kinds of them and make sure that you have it on all aspects. Even if you're a back-end developer, if you're tackling a refactor, you need to look at your front-end tests and make sure that they cover everything or cover as much stuff as you can. This is a good time to review the functionality of what you're trying to refactor. Sometimes, especially if you're a back-end developer, you might not know exactly how the front-end works, how people interact with it on a day-to-day basis. This is just good practice as a developer generally, but this is a good time to do it. To sit down with the product manager or the customer support person or even the senior developer that's been there longer and make sure that you know exactly how this code is supposed to work and exactly what it's supposed to do. It's hard to refactor and keep that end goal keeping that the same if you don't know what it is. So you want to make sure that you have a really solid foundation in what you're refactoring. And then make sure you brush up on code style. Code style is really important to lots of people to make sure that especially if you're working on a team that everyone is sort of on the same page and this is a good chance to make sure that you know what your team expects, what sort of trends you're moving towards. If your team is really trying to move logic out of the controller into the model, you don't want to do a giant refactor that puts all of the logic back in the controller. It might be cleaner code, but it's not really where your team is going. So those are the kinds of things to keep in mind while you're doing this. I want to point out at this point, if you're taking a refactor of application code, you don't want to change tests while you're refactoring. This is a pretty important point that was not fully explained to me on one of the big refactors that I was asked to do because I was a noob and I didn't think to ask. If you're keeping your production functionality the same for your end users, your test is going to be your safety net to make sure that you're actually keeping that. You're not breaking anything in your refactor, you're not making anything totally different. So if you change your test midway, you don't have that safety net anymore. Your safety net has been moved and shifted and maybe now it's all the way over there and you're still dangling from a tree over here. So that's a good way to get your test coverage up to snuff is do it beforehand, make a separate commit, get that pushed through. Now you have a better test coverage, which is great, just generally, even if you never do a refactor, but that makes sure that you have that safety net during your refactor. And then like I mentioned before, you can always refactor tests. So if you are refactoring your tests, the flip side is true. Don't go changing your application code while you're refactoring your tests. You want the same test to pass the beginning, that pass at the end. And if you've changed them, you can't be 100% certain you didn't screw anything up. Hopefully you didn't, but you never know. So once you have all of this preparation done, you're all set, you have your test coverage done, that's all merged in and everything looks great. The next thing I like to do is go through the code that you're targeting and annotate it. So this is basically reading through and commenting anything that stands out to you was weird or bad. This is, you know, some people like comment, some people don't. These comments will go away once you're done, but this is a good way to go through and highlight to yourself. This is the stuff that looks weird to me. This is the stuff that looks bad. This is what I want to fix. One thing that I like to do sometimes, this doesn't work always, is consider copying outside code that gets used in the code that you're looking at. So if you're in a controller and your code is relying pretty heavily on this validation that gets done in your model, you might want to consider copying that validation over. If you're having trouble remembering what it does, or you're having trouble seeing how all the pieces fit together, you can put it in a comment, you can take it out when you're done. You don't have to worry about making your code undry or whatever, but it sort of helps to see everything in front of you all at once. It might even help you figure out that maybe that logic you just copied over belonged in the controller the whole time. Seeing that all together can sort of help you with that. This doesn't always apply, but I think that's an important thing, is copying isn't always bad. You don't want to keep it necessarily, but you can always copy intermediate. And then the last part is rename variables and methods. Some of these will get renamed permanently. We'll talk about this more later. But some of these are just for clarity. For an example, say you're going through some controller code and you keep running into this method called clearParams. You're like, what does clearParams mean? Maybe that's not a good name. Maybe at the end of the refactor, you decide it really should be called cleanParams or strongParams or some other standard name. So you're like, what is this? I don't really know. And then you go to that method and you realize, oh, it's actually removing all this crazy stuff from the prams that I didn't want. So you go through the code that you're refactoring and you rename all the instances of clearParams to fixParams by removing all that crazy stuff. So every time you see it, you know exactly what's happening. You don't have to remember, oh, wait, what was clearParams again? You know, it's fixing the prams by removing all the crazy stuff. That's obviously too long of a name to keep in your code. You don't want to keep that there forever. But it's a good chance to give yourself an explanation in line that doesn't interrupt the flow of you understanding the code while you're working on it. Understanding is the key. That's what you're trying to get at at the end of a refactor. So making sure that you're understanding what you're reading, even if it doesn't stay that way forever. Allow yourself to sort of experiment with crazy, weird stuff. You can always change it. So what are the kinds of things you want to look for when you're annotating? Some people call them code smells. I like to call them code messes. Like your dogmex little messes. You still love it. It's still lovable. What looks redundant? We don't like redundancy in the Ruby world or in any coding world, I hope. What is too complex? Your code should be clear and non-repetitive. What parts of the code took more than one to two reads to understand? Again, this is a time where you should trust your gut. Don't just assume that because it took you four reads to understand what was happening, that you're just a dummy and you didn't understand it. Probably it was poorly written. And maybe even if at the end you decide that nothing about that can be changed and it really just is a complicated concept, maybe it needs more inline commenting. So that's just the thing to note to yourself. Do you see any methods that look bloating? We don't need to go all the way crazy down and be like Unix and have only one action per method, but you want to get closer to that. You don't want to have a method that's 20 gazillion lines long because what would you even name that? What would it do? Mark those kinds of things. And then, does any of it violate your code standards? This is pretty simple. There are lots of tools that can help you find these things, but this is a good time to mark those things down. So once you've pinpointed all these things, you have nice annotated code, it's all marked up. How do you do it? You go through and clean up your code. This is the part that's really the meat of actually doing the refactor and this is the part that might seem the most overwhelming if you've never done it before. So I like to recommend people don't start it. You start at the top usually if you're reading through a file. If once you've annotated it, you know where all the problems are. You don't need to start at the top again. Start at the easiest thing for you to do. Then, if that's all you've done, you've still made an improvement. So if you have a bunch of instances of, for example, someone using single letter variable names, those are pretty easy. Really low hanging fruit. You can go through and clean all of those up, make that a commit. If that's all you do, that's a great refactor. You've just made the code more easily understood for other people who come after you. So tackle those small things first before working your way up to more complex things. Focus on one method at a time. Make sure that nothing is repeated. Those are pretty easy things to do. Just pull stuff out, making methods, smaller methods, those kinds of things. I mentioned names before. This is a good time to make sure that your method names make sense. Maybe you don't wanna leave fixed crazy programs by removing all the ugly stuff, but maybe you wanna rename it clean programs or strong programs. Something that is more indicative of what it's actually doing. Fix those style issues I mentioned. That's a pretty easy one. Most of us can handle that even if you don't necessarily agree with hash rockets versus not hash rockets. You can do it. We all know how to do that. As we get down the list here, things get slightly more complicated. Simplifying conditionals is one of those things that is really important to making code readable and is one of the harder things for people to do. If you have to sit down and draw some truth tables or do Venn diagrams, stop it. Take the time and do it. One thing I like to do with crazy looking conditionals is to break it down. If you have a bunch of like ands and oars or like nested things, break each one down so that you have nothing nested and nothing anded. Maybe you're repeating all of your crazy conditionals, but it helps you get a handle on what actually is happening. And then if you need to, you can build it back up and combine things as necessary. But it might help you see, oh, I didn't actually need to check this four different times because I've in effect checked it this one time. So now you've gotten rid of half of them. And then the probably the most complex thing here is making sure that your code lives where it truly belongs. Some of this is team style preferences. Like I mentioned, some of it, like for example, my team is moving a lot of our front end logic into JavaScript, which we all know how DHH feels about that. And if that's where you're going, maybe that's what you need to do. But if there's not an overwhelming preference for these kinds of things, then this is your time to sit and think critically about, does this actually make sense here? Is this controller code actually important for a specific controller action? Or is it integral to my model? Does this actually need to be moved into my model? Is this something, is this really complex looking method? Is this something that really needs to live in my model? Or is this something that I can pull out and put into an asynchronous job so that I don't have to wait for my model to respond for this crazy looking thing that I'm having trouble simplifying? Things like that are important things to think about when you're in, and this is the kind of thing too where if you get to this point and you're like, I don't feel like this really belongs here but I'm not sure where it belongs. That's a good chance to pull in someone else from your team. If your team does pair programming, that's a good chance to sit down and say, I feel weird about this method being in here. It just, something seems off to me. Where do you think it could go? Or sit down with someone and maybe try it over here. See how it goes, see how many tests fail once you move it to the model. See how many tests fail when you move it to your backbone model. Sort of feel it out, bounce ideas off someone else to help you get a sense of where something actually truly should live. So now that we've gone through some ideas on how to actually approach doing it, I have a very brief case study. So obviously showing a whole refactor on a little slide in a 30 minute talk is not going to be very practical. So what I did was I pulled out a small snippet of an actual refactor that I did recently. And I'm just gonna walk through it to sort of illustrate the method that I was just talking about in some of the concepts. But this will mostly just look like I'm cleaning up some code, which I mean to be fair, that's what refactoring actually is. But none of this should be too mind blowing, which is also good because it's really not that mind blowing. So this code was part of our signup controller and we were trying to refactor a very small bit of it and it snowballed into a very big refactor. This was from the new method of someone signing up for a free trial. So you can kind of just look at the shape of this code and you can kind of tell it looks bad. Like the first line is really long. Does it really need to be that long? Do we really need to have like four methods chained together? You can see we have a bunch of like nested stuff with like unless and then if and then if not, just basically unless, you're like, I don't know. And then we can see some single letter variables which are always horrible. So we go through and annotate it. So here's my annotations I made for this particular bit of code. You know, you can see why is this so long? Another bit is why are we having param? Like what are param signup? Object oriented programming doesn't really tell me like what is a signup? We didn't have a signup model in this particular project. So I was like, I don't really know what that is. Takes me a lot of like that was one of those I had to go and pull extra code in my comments to see what exactly this was passing me. We got some nested conditionals, single letter variables. We're assigning this you variable twice. What are we doing that for? So this is all pretty ugly. So when I was addressing these comments, how did I go through here? So from easiest to hardest in this particular case, easiest thing to do is to fix the names. So we took these single variable, single letter variables and made them actually useful. So these use are actually users and the a's are actually accounts. We don't need the use in the a's, that's stupid. We simplified these conditionals. So this is when I broke it all out and then built it back together so that we don't have unlesses and elses and double nested things. And then probably the biggest thing that we did here is I went back to this param signup form and well the first thing was to take this like triple chained method at the top here that like does some crazy thing with the email and like only downcases the first 100 letters. I still don't know why that was there. But what we did was we changed it so that it was just a front end validation which is really where this should have belonged to the whole time, making sure that someone is actually giving us a valid email address and that we don't need to do anything to it once the time, by the time it gets to us. And then the very last thing was going back to the form that was generating these signup params, realizing that what it was actually doing was creating a user object and just renaming it params user. As a Rails community, params user is something that you have a built in understanding of what that means. Params for a user, that's cool. Params for a signup, I didn't know what that was. But params for a user, I'm like oh okay, here's some user params. Maybe we use strong params on it, maybe we don't. But this is gonna create me a user object which is what in fact this was doing. So renaming that what was coming out of that form made it sort of more Rails-y and easier for other people to understand. So once I got through refactoring this is what it looked like. You can see that I also added in some explanatory comments which, for things that I thought were confusing for someone new to read but that we couldn't necessarily change like we were checking this demo account. I didn't really know what that was or like why we were doing this. So I put in some comments once I found out. You can see we renamed the params here. We made the variables useful variables instead of A and E and U. And then the condition was broken down here so that it's nothing is nested and nothing is using if not or unless which is always a little bit harder to understand. In this case you can see you might notice that we're checking if existing user twice which some people might look at that and be like oh hey that's not dry. In this particular case we decided that that check wasn't too expensive and that the readability of this part of the code trumped whatever we might lose by not repeating that. We thought the double nested conditional was a little too complicated in that repeating this one thing once was fine for our team. So that's something else is you don't always necessarily need to adhere to style super, super hard if readability is your goal. So here's some other tips. Now that we've gone through this briefcase study. Test frequently. This is why you have those tests. This is why you went through and added those tests. Make sure you're also testing manually if this is something on the front end or something that you can test manually. This is why you had to understand what the functionality was. Your tests don't cover everything even if you magically have 100% test coverage which if you say you do I don't believe you. Again don't change your tests while refactoring. Please don't do it. I did it. Learn from my mistakes. It goes poorly. Commit frequently. Once you have a green, once you make a change so say you changed all those variable names to something that made more sense. Run your tests just to make sure that you didn't mess anything up. Once they're green, commit. Push it. Open up a port cross. Say look what I did. Then you already have a refactor. So make sure you're committing frequently. That way if you do you break your tests it's easier to go back to the last thing they were green. And I mentioned really your tools like RuboCop or Reek can help you find places in your code that are breaking style. Find places in your code that you may have missed that are code smelly. To give you something to start on. Those kinds of things can help you like oh this method is way too long. You're like oh I hadn't even noticed. It looked totally normal to me. And then consider pausing after a logical group of changes. So after your test pass in your, in the example I was giving of changing the variable names. After that passes say maybe you decide you wanna deploy it. It only took a day. You did it, got reviewed, it got merged. Deploy. Cool. Refactor deployed. Awesome. Done. One thing with this last bit though is when you're planning what to, what to group together or what to deploy together. Sometimes you run into two different ways to do it that sometimes you plan, sometimes they don't work. So on one hand you have this lovely turtle who is trying to slow and steady, creep his way through, do little bits at a time and incrementally change stuff. And sometimes this is great. Sometimes this is what you need to do. Sometimes the structure of the management on your team is such that it's easier to just grab little bits of time here and there instead of trying to get a whole sprint prioritized for a refactor. So sometimes this is great. You don't always make as fast of progress but that's okay, it's still better than nothing. But on the other hand sometimes you work on a team where making everyone have a heads down like we're gonna work for an entire solid week and everyone's gonna refactor. Maybe that's more useful for your team and that'll get all the changes done at once. That's cool. If you work on that team kudos for you. And just to go back to the slow and steady for a second. Sometimes it's easier too even if you don't have maybe express permission to like I'm going to refactor. If you're touching a piece of code sometimes people like to talk about the idea of leaving code better when you left it than when you found it. And this is a good way to do that. Not only can you go in and fix a bug, it's a one line fix, you make sure your tests pass and then you're like, well, I'm in here. This looks a little bit weird. That's a good chance to just say you have a 10 line method and you fixed a bug on one of the lines. It took you two minutes. You can totally spend another hour maybe fixing up that method a little bit and then it's nicer for the next person that comes along. Didn't take you that long. You didn't need to have to wait for a refactor, anything like that. Those are really easy things that even brand new beginners can do. But one point also I sort of mentioned this with our example is that sometimes if you try to start a small change, it can really easily snowball. You pull on one string and the whole thing comes apart. Can really easily snowball into refactoring a whole bunch of stuff. So keep that in mind. If that starts happening and you're not prepared for it, it's fine. Commit the stuff or stash your changes and go back. Maybe wait till you have time to pair with someone more senior than you if it's something that's overwhelming. That can always happen. It doesn't mean you're doing it wrong. Just means that code is complicated. Code is complicated. There's no way around it. A few other things to remember. Refactoring's not scary. We do it all the time. We don't even always call it refactoring. It's a good way to learn new design patterns. The refactor that actually got me that Pop-Tort box was a refactor of a DSL. I had never worked in DSL. I didn't know that DSL stood for domain-specific language. I had no idea what good practices work for that. So it was a good chance for me to go and read about these things, read about how the code was structured, read about all the ways in which the code I was taking was breaking all of those rules and how to not break the rules in the end product. And I wouldn't have learned any of that stuff if I had just continued working in the code the way that it was. It's a good way to learn your code base not only because you're doing the things at the beginning that we talked about, like fixing your tests and making sure you understand the functionality. But this is the sort of stuff that you can really sit down with. Maybe the person that wrote that code. Maybe that person's on a round or it's a whole bunch of people, but the person who most recently did a lot in that code or the most senior dev on your team and say, can you walk me through this? I'm having some trouble understanding what's going on in here. And that's a good way to sort of get a refactor started and also to understand really, really deeply what is happening in this code, what exactly is happening in here, why is it structured the way that it's structured and how can we make it better? Again, tests can be refactored too. If you're a tester, if you're not a tester, you see your tests and you're like, I don't understand what these tests are testing. That's a good chance to go back and refactor your tests even if it's just the descriptions of your R specs. That's important. Those are all important things. These are all refactors, by the way. Some of these things sound really small, but like I said, refactoring's not scary. And then a good thing to do, especially for beginners, is to go back and refactor your old projects, the first things that you coded. This might sound really scary because probably the first things that you coded, now that you've been coding for a while, you'd probably read them and think, oh, why did I do that? But this is a great example of you're probably gonna read some other people's code who have been coding for twice as long as you have and go, why did you do that? So this is a good practice. If you can do it to yourself, you can do it to other people. So now that you've learned all of these things about refactoring, you can go forth and refactor. Any questions?