 Welcome everyone. So I'm Josh Karajewski and I am excited to talk to you today about the Art of Refactoring. I'm going to do a combination of things. I'll show just a couple of slides. I'm hoping to actually get into the IDE and do a few things with actual, you know, live refactoring. And I just want to also just share some stories with you. So also I hope to leave time for Q&A. How many of you here regularly refactor code? Okay. How many of you here would like to refactor but don't do it very often? Okay. How many of you here just aren't allowed or don't think you have the time to refactor? Because you're always so busy. Okay, so there's some of that. Okay, so the basics. The basics of refactoring first of all, right? What is refactoring? It's actually that term is used in all sorts of ways. Let's refactor our organization, right? We hear stuff like that. I tend to think of refactoring as being mostly about code, mostly about improving the design of the code. And what needs to be true when you improve the design of the code? I had this happen in my workshop. People don't like to answer. So what happens is you have to preserve the behavior of the code, right? If you're changing the design of the tax calculations, well the tax calculations still need to yield the same results. But the design is different. So good refactoring, of course, preserves the design. We have all sorts of ways of discussing it. We say behavior preserving transformations, right? We're transforming things in the design, but we're preserving behavior. It's also just other terms, but I won't bore you with those. So behavior preserving transformations, think of it that way. Now there are tools that help us refactor. How many of you here use an automated refactoring tool? Okay, only a few hands. Only a few hands, right? So the rest of you may be using a language where you don't have good tools, right? There aren't great tools for some languages. Other languages do have good tools. How many of you here regularly program, for example, in Java? Okay. So for Java, for example, there are very, very good tools. I shouldn't say very, very good. There are good tools. They've kind of leveled off in how great they are. C-sharp, the same thing. Very good tools for C-sharp. Resharper is one of those tools. Why am I talking about tools? In the 90s when I programmed a whole lot on Wall Street, I refactored a lot, but I did it without tools. I actually did it without tests. I did it without automated tests. You can refactor without automated tests. You just have to do it carefully. The automated tools make it even safer. One thing I talk about a lot in preserving behavior, right? Behavior preserving transformations, you've got to do it safely. How do you do it safely? Well, you test a lot or you use tools that you truly trust. You've used them again and again. You trust them, but you got to have that set of safe ways of working if you're going to do this, especially without tests. You can refactor with or without tests. All right. So one of the things we see typically when we're in a context where we might refactor is this tension, right? This tension between building it right and shipping it fast. And that tends to be a tension that's present between development and business, right? Build it right or ship it fast. Do you experience that? Is that what you have out there? Do you tend to build it fast or build it right or do you tend to ship it fast more often? How many of you here ship it fast more often? Yeah. Usually that's the case. You just build it. You get it out there. But you should be able to come back and have time to build it right. If you don't take that time, what happens? Technical debt accumulates. You get more and more and more technical debt. It becomes harder and slower to change the code. So it becomes imperative to refactor and to find time for it. It's not a luxury to refactor. This here is one of the scariest things I've ever filmed. It was my own client's code in Toronto, Canada. I asked them for the longest method in their most important system. This was a system we were helping them. We were about to help them start working on. It had an enormous amount of complexity. This was a 28-page function. I filmed it in a hotel lobby. We taped it together in the hotel bar and then I rolled it out. I call it the scroll because you roll it up and then you just roll it out. This video helped me convince the managers that we have a problem. How many of you here would love to convince your management to actually fix some legacy code, to actually improve it, to clean up the technical debt? I've heard a lot of people say, it's great, you're teaching refactoring, but how do I convince my manager to refactor? I hear this all the time. I've heard it too. I've experienced it too. I've had to convince people, so I've made videos like this. In fact, one of my own clients won up to me on this, came up with an even better idea. They printed out the entire scroll of the longest method, then they rolled it up and then they asked the manager to grab the end of the roll and walk with it and keep walking with it and keep walking with it until they came to the end. If the manager could viscerally feel the complexity, the technical debt walk, walk of shame. So it helps to be able to convince folks, oh, and by the way, that particular project, it had a whole list of features they wanted to work on. We were able to kill those features and not build them and say, nope, nope, nope. You've got 10 years of legacy C++ code here. We are going to pay down the technical debt. We're going to refactor this code. We're going to refactor it. We're also going to put a whole bunch of automated tests around it. And we did. We started off with a bunch of high level tests that we actually used a screen recording tool, a piece of junk, but they already bought it for $100,000. So we used that to basically write a whole bunch of smoke tests and automate them. That gave us some confidence that things still work so we could go back into the code and start to actually, you know, do some low level changes. Okay, so one of the things I go after, you know, is fewer moving parts when I'm doing refactoring. Fewer moving parts. Any of you ever had this experience here with shower curtain? I kind of bristle a little bit at bad design. And the shower curtain rings. I mean, that picture there, I'm embarrassed to say that's from my own house. It got purchased, it wasn't my decision, but the point is that sucks. You got moving parts and things break and it doesn't work well. Whereas this wonderful concept here is awesome. I see this in more and more hotels around the world and I love it. Every time I see it, I'm like, oh, great, simple design. I love it. This is important for refactoring, right? Because with refactoring, we're trying to improve the design of the code. We're trying to simplify it. One of the great ways to simplify it is to reduce the number of moving parts like we did here. So you always have to be thinking about how to make things simpler. Sometimes there's a variety of ways you can do that, right? Sometimes it's as bold as swapping out a whole bunch of code that used to exist and using the library that already does the work, right? That's a bigger change, but it might actually lead to much simpler design. In other cases, you just see the way the code was written was not very simple and you find a way to simplify it, often after reflecting or collaborating with others on how to better refactor it. Another example, someone actually purchased for me this complicated kettle with all these buttons and ability to set temperature at a certain number and I'm like, you know, we returned it because all I need is to hot water or not. Hot water, one button, done. Simple design. Simple design was one of the 12 extreme programming practices, by the way. The concept of creating simple design was there from the beginning. Now, one of the things I've learned over the years is that I can't make all of my code simple and I'm not going to do it. In fact, it just doesn't pay dividends, right? You end up with a lot of code over the years and you do your best to work with it, refactor it, but it can be a losing battle to make everything a perfect design, to make everything crystal clear and simple. Some code just works and we view it as plumbing. If we look at the system overall, for example, imagine this box here is the system. Some of the code in the box is more important than other code in the box. It's not equal opportunity. If this code here is the code that makes me money, well, then it's going to have a far better design than the code over there, which is just some plumbing code. I'm not saying I don't have a decent design for those things. I'm not saying it crashes and has lots of bugs. It doesn't. But I could radically refactor this code here. I can look at it and go, yeah, there are some smells there. But the art of refactoring is not about making everything perfect. I think you have to make some good decisions. I use a term you may not have heard before, but you've heard the term hacker, right? Hacker? In the good sense of hacker? The kind of hacker that's always looking for a creative solution? You've heard entrepreneur, right? There's a term called hackerpreneur. A hackerpreneur is someone who's thinking about business while hacking. For me, I run my own business. I've built a product. It sold millions of dollars out there in the marketplace. In my own system, I know we all know where the code is that makes the money. That is an area that has very, very good design. It has the best tests we've written. It has a lot of attention. The art of refactoring, remember, you can't make it all perfect. I want to switch now to actually doing some live refactoring for you so we can get a little more serious about this stuff. First off, I'll just run my tests. Make sure everything's good. Do you see that? Okay. Great. What I want to do now is just demonstrate some refactoring techniques. This is a body of code here that is intentionally very smelly. It's filled with code smells. I call it smellections. It is a collection library that I wrote personally. It's the smelliest collection classes on Planet Earth. I dare you to find a worse designed collection library. It does work. We've got a list, we've got a set, and we've got a map. The problem is that there's massive amounts of duplication. If I were to compare a couple of these, and I just say, let's just compare these files, there is a lot of duplication. You see those methods there. I'll make this bigger for a minute. The ad methods are practically identical. The contains methods practically identical. Remove practically identical. Look at this. We have get in the list class, and in the set class, we've got get element app. They do the exact same thing, but they have different names. What else? So you see lots of duplication in this code that really shouldn't exist. It's not buying us anything. So there are lots of smells. Map in particular has some pretty bad things in it. It, boy, it grows its arrays when you add an element, or you add another item to your map, and the map's not big enough. It has to grow itself. So there's low level code here to make the keys bigger and make the values bigger and so forth. But, you know, really, the lists could do that. Lists already know how to grow themselves. They already have that low level code, right? They do this similar thing if new size, oh my God, wait a minute. This is, that code is awful. Can you read, can you understand what this code does? Or you could read it, right? And you could understand it, but that's really not your job. Your job is to make this code easily readable and understandable, so that someone doesn't even have to give it a fraction of a second to understand how it does what it does, right? We have to struggle right now to know how it does what. We know it adds. We know it adds an element to a list. We know what it does. But we don't know the how. And I want to know the how. So you know what, let's fix it. First of all, I'm using IntelliJ. I'll have this little thing called invert if condition. Oh, look at that. We just inverted the if, ran all the tests, 48 tests just ran. That's nice, right? Now, you might hate me, but I don't use curly braces for one liners. So, oh, a fan. That's rare. Thank you. I'll pay you later. What can I do next? This is why I would love it if you participate. I know you might be shy. But tell me what to do. I want to make this code readable. Move the code. Which code? Expanding the list. Expanding the list. So this code here? What should I move it to? Maybe one of these, extract method. What is it going to do inside of that if? Resize. Okay. Resize. That worked. That was nice. What about this stuff here? This doesn't look like it's at the same level of abstraction, right? We have this low level stuff with new size. Inline. Inline what? Inline size plus, oh, so inline new size? Okay. I'll inline it. Now what? Remove the curly braces. I'll take it. Now what? Anything else? I want to back up one minute. I want to back up one second here. I just want to show you something. Let's just back up. We're on the test. And what if I were to just extract this in here into something that was a little more readable, right? I'll refactor it to should grow, right? And then it's a little more readable, right? It says if should grow new size, resize. Is that good? Is that better? Does it read like English now? Do you ever say things like if should grow new size? Do you do that? I don't. In fact, it's not good. Let's undo it. It wasn't a good refactoring. And as we did before, we do want to inline this first. Then after inlining it, by the way, I'm not going to run the test after inlining because I trust it. I know it's working. I don't even need to run the test for that. I'll run this here and now I'll say should grow. Now that is starting to look a little more like English. If read only return, if should grow resize, we'll get rid of the unnecessary curlies. We are professionals and we have tests. That's good, right? Anything else to make this readable? So maybe convert this into add new element. Thank you. Add new elements. Call it that for lack of a better. Okay, so we just made three private methods. Oh, no. Is the performance going to be a problem now? Does anyone think that's going to be a problem? Show of hands, performance issues with this? The compiler is going to handle that, right? We can actually show that with a profiler that this has absolutely no impact on the performance. So don't use that as an excuse for not extracting methods. This is our job. This is probably one of the most important jobs we have in refactoring is to make the code readable. Ken Beck calls this a composed method. It's a method composed of other calls to other methods. And it's readable. And it's at the same level of abstraction. There aren't any low level details mixed in with high level details. Everything's about the same level of detail. Now is the design awesome yet? Not quite. Not quite by a long shot. This is smell collections after all. It's the smelliest collection classes on planet earth. There's a lot more to do. I just want to point out something here. Look at this read only flag, which of course, well, set has a read only flag and map has a read only flag. Oh my God, there's lots of problems. But just let's focus on list for a minute here. Read only that's one occurrence of read only. Then we have read only referred to here. That's two. Then read only referred to again is here. That's three. And then read only again referred to in the set. That's four occurrences. And then we have the actual read only setter five kind of occurrences of read only. In code, that's not very long to begin with, right? There's not many lines of code here. What do we have? We have 99 lines of code. And we're referring to this week. We're doing read only type stuff throughout this class. Now how often do you make a collection read only? How often do you actually go and say, Oh, my collection class shouldn't be modified. I'm going to make it read only. Maybe not that often. So what am I doing here? This design has mixed things together, right? There's the core logic of what it does. What a list does is you add or remove elements. That's core. But then I have this some of the time stuff, this optional stuff that's mixed in with the code. So I have core mixed with optional. That's a smell. There's something wrong there. I don't want that read only polluting what is meant to be a simple core function of list, adding and removing. What could I do instead, potentially, to allow a list to be made read only, optionally? Anyone? A new object? Which would do what? A read only list. So I could make a read only list. Would that be, how would I do that? Would it be a subclass? Would it be maybe another kind of object? Anyone? We might use object composition. And we might say that here's the list and I'm going to wrap it. We would call it a protection proxy in design patterns lingo. But we would protect it with a read only list protection, a wrapper. If you go to collections class and Java collections dot read only list, you pass in a list, it returns you a read only list. So there's a lot of things that we need as our guide as to what to improve in the design. Part of that is getting a good nose for what poor design looks like. We call this code smells. Code smells, there's a whole catalog of these things. Martin Fowler here and Kent Beck, they wrote a chapter in their book on the book refactoring that Martin wrote. There's a chapter on code smells listing a whole bunch of different smells. Things that are more obvious like long methods are a smell, down to more less obvious things like refused to be quest, inheriting something that you may not want to inherit. For example, let's look at the map class here. Oh my God. Is this really here? Do nothing because user must input key and value. Oh my God. We have a map extending from abstract collection. It seems that abstract collection defines the ad method or no, it doesn't define the collection defines the ad method. And so map is compelled to have an ad method that does nothing and look it's public. Oh my God. Refused to be quest. We've inherited this thing, let's do nothing with it now. Terrible smell. There are a lot of smells here. In fact, I test drove every smell in. I had a list of smells and as I created the code I deliberately put the smells in place. But let's just go back to the art of refactoring. One of the things I can do to simplify map is to have it work with lists. Let me just have it work with lists. So I'm just going to go here, what keys and values are these object arrays? If I make them list it's going to be a lot simpler, right? So let's just do it. One of the things you need to do in refactoring is just make the changes that you need to make. Make them happen. Don't mess around. I'll just say new list and get that going here. And yes, I want the smell elections list. Okay. So no, I've got some red on the side here. I guess I'll start going through it and fixing these things. I am teaching you the art of refactoring here after all. So keys.sub i, what would I change that to? Keys.set, maybe? Oh, is it equal? Keys.get, sorry. Keys.get and then change that to an i, right? Pretty good, right? Can I get any kind of applause for fixing that compilation error? Okay. Thank you. And I'm going to keep going as we do, right? We just sort of go through and make the next change, right? Do you see the art of refactoring here? Do you see it in action? No, you don't. No. This is horrible, folks. I'm going to undo. What I just did was a travesty against the whole art of refactoring. Please don't do this at home. It's a very bad idea. What I've done is I've created non-compiling code by taking a giant step. I took a really big step. I went from saying, I don't want these things to be object arrays. I want them to be lists and I just made them lists and I broke the code. It doesn't compile. It won't compile for several minutes while I stay focused, supposedly refactoring, fixing every problem. That's not a good way to refactor. That's not safe. You want to refactor safely. Let's undo this. Let's get back to our... Let's do what I call a graceful retreat and get back to the green bar. Now I feel good again. What could we do instead? First of all, even if we did this, and I'm not going to do it, but even if we just said, let's go after one thing at a time, right? Now I have fewer red marks. I still have too many for good refactoring technique, but at least I have the problem. That would be a little better, but it's still not good refactoring technique. Just because I'm paranoid, I'll run the test again. I feel good. Values. I'll decide I'm only going to work on values. What I would like to do in 2016, for God's sakes, is right-click and say refactor encapsulate fields. Unfortunately, unless I'm really mistaken, it can't do it. Normally you can encapsulate a field, but it's almost like you can only encapsulate a simple field. If I were to actually do this, let's see what it's going to do. I don't want to do this, but I'll do it anyway and just see what it does. What did it do? Yeah, you see, what it's done is it's created a get values. That's all it's done. That's not what I want at all. What I want is for it to, I want to encapsulate the syntax for getting or setting anything out of values. So I'll undo that and rerun my tests. How much time do we have here left? Okay, 10 minutes. That's fine. What I'm going to do instead is I'm going to say, all right, fine. Every single place where I'm doing this, let's say I highlight that. I can extract a method and I could say get value, maybe I'll call it get value at. It's detected four code fragments in this file. It can be replaced. Yep, let's replace it there and there and there and there looks good too. Oh, so I immediately see there is a problem. So where is this problem? I think the problem is in the abstract collection. Sorry, it says that there's a problem in map. Oh, here. Get value at. Not happy with this. This should have been a set. So I screwed up. Let's do this again. So I undid that and when I undo, by the way, it undid everything. You got to be really careful. After you do a refactoring, don't start typing on the code because then you can't undo. So I'm very careful after doing an automated refactoring to see that, you know, if it broke something, I retreat gracefully. And I approach it again and say, you know, all right, this really should have been a set value at. So maybe I'll start with the setters. I'll say let's convert this into a set value at. Okay. And that's not going to work either, is it? Because I want the whole thing. So I got to highlight this and say, this is what I really want. Set value at, right? Let's just try that. It's detected two other code fragments. So we'll see what it does. Okay, here's another place. It looks like I should change it here. It's going to turn out to not be correct, but I'll say yes, because it's easily fooled. We'll do that, try it again, and it passes. It passes inside of these tests for the moment. Okay. So here's set value at. I've begun to encapsulate all occurrences of calls to values. Now I need to do the getters. So any place where we get a value out, for example, right here, right here's value sub i, that should have been our getter, get value at. It'll find the other occurrences. I'll replace them all. Why is it doing this one? I want values. Did I just do keys? I undo that. I always undo when I'm unsure. Do that again. Oh, so it did return. It did do this one. Son of a gun. I undid it. Alright, so it should have undone this. It should have done this one. Let's do this one here. Get value at sub i and here. No? Did I mess that up? What did I do wrong? Oh, yes. That messed that up completely, didn't it? Geez. So I do want to, and now this is a case where I screwed myself up because I can't undo. Well, I did. Wow. That was surprising. That was strange because it didn't really undo what I wanted it to undo. Let's try it again. So I don't want it passing in the object. Why is it passing in the object here? Am I doing something bizarre? What I want is a get value, is it just a simple get value at sub i? And I've always been able to do this, so I'm confused. Why does it think it needs to pass in an object? Totally strange, but just to be on the safe side, I'll simply write it myself. Private object get value at, it's going to take an int and it's just going to return values sub i. Is that what I want? And then I'll just change this to get value at passing the i. And I'll just test that, because that's one little change. It seemed to work. So let's just replace it here. I shouldn't have to do this, by the way. This should have been done by the tool, but the tool was misbehaving. Alright, so that's that. Let's look for any other occurrences of values. There's one, values. So this one should also be a get value at. Get value at, the index where key found. Okay, good. And I think that's going to do it. So I'm now ready to make my change. I'm going to say list, and I'll say new list. And I can't run the test, but I can run the test. What I see is there is three places where we have problems. Here's the first one. Values equals new values. Oh, look at this. So this is stuff where I'm trying to grow the values array. I don't need to do that anymore. I can get rid of all occurrences of that. I'm down to two compilation errors. Okay. Ah, here is set value at. This is no longer what we want. Now we have the values list, and we can say set. And for set we pass in i and the values. Is that right? Or is it the other way around? I think that's right. Finally, here this get value at, and this would be just values.get sub i. And we'll rerun the tests. I got a problem. Aha, I knew we'd have this problem. Look, abstract collection's unhappy. m.value sub i. That's no good anymore, right? Now it's, so this is really bad design, by the way, right? Because it's referring to a protected list. The abstract collection is reaching into the map and calling the values field. That's poor encapsulation, right? If you studied encapsulation, so boy, we've got to fix that. m.what should it be? Get value at sub i. That should be right, right? I do something wrong. Oh, that's probably, so that's that's no good either. So this is a problem of it being private when I made it private. So I'll temporarily make it protected. But it's a little dangerous right now, so I want to be careful I can get back to the green bar as quickly as possible. Okay, now it all runs. Oh, test failure. Array index out of bounds. I'll save you the, I'll save you, this was wrong. We did this earlier when we were encapsulating, and it wasn't actually right. Even though the test passed, it wasn't right. This should have been values.add the value. So we made a mistake along the way, and that's going to happen. But then we can get back to a green bar. So we just successfully encapsulated the values field, which again was an object array. We made it a list. I tried to show you how to do it safely, right? One step at a time, undoing quite a bit when things didn't go right. It was very real-worldish because I created a bug. Of course my tests were there to help catch me, which was wonderful. That's the safety net, right? And I took really little steps, small steps, small safe steps, always retreating when there's a problem, and then going forward when I when I could. Part of that's the order of refactoring, right? So I showed you two things today. You know, we cleaned up the add method of the list to make it human-readable, so that other developers could quickly understand how it does what it does and can change it if necessary. We also talked about the sum-of-the-time logic, not making this a very simple design. And then for map we saw the technique for not the wrong technique for improving its design, right? Taking too big of a step instead of taking small safe steps. You saw that process, right? You saw me using the automated refactoring tools. Sometimes they work, sometimes they don't. You have to kind of be careful with them, right? They're not perfect, but you get to know them over time and you know what they can and cannot do, and that helps. I myself was a bit surprised today by certain behavior, but maybe I need the latest version of the idea from tele-J. So that was our time. Does anyone have any final quick questions before we conclude? They're showing me the out-of-time... Oh, we do have five minutes for questions. Five minutes for questions. Anyone with a question? Yes? So what's a good way to convince people or what's a good argument that you've used for management or higher-ups when people say I don't have time to refactor, but we know that it's important or useful or relevant or value-added. What's one of the best ways that you've helped convince people to carve out time to refactor? Well, first of all, I would say that going back to this presentation here, you know, showing visualizing the state of affairs for them, visualizing how bad it is, can help management see, you know, how utterly misguided their current way is. Right? Explaining to them that this is actually... I mean, this was interesting because this was this client in Toronto we had. This was some code from their most important system, which was 10 years old, and this was a company that was leading the field in mass spectrometry, whatever, you know, they make their own mass spectrometers. They were the leading provider of this stuff. Their competitor took out an advertisement showing someone holding their head like this, and in giant font it said, don't tolerate software crashes. That was the code crashing. And so it was, that was relatively easy for us to say, you know, hey management, you know, you're really making bad decisions here. You need to start refactoring and improving the design of this old code because people aren't happy. It's not a competitive advantage. So I tie it, I tend to tie it to the business and explain how, how can we refactor to be more successful in business. And again, the focus here on where do we make our money, right, because if this is where we make our money, this is what clients love about our product. You know, when clients buy my e-learning, they don't buy it for our user registration code, which registers a student in the database, they buy it for the other things it does, which hopefully are down in here. You know, so I think the other thing I'd say is that you want to refactoring all the time, constantly cleaning as you work. You're constantly cleaning as you work. There are going to be some things that are too big to go after in a short period of time. And that's where you're going to have to really reason with management to say, you know what, if we change this design over this new way, here's what business capabilities we're going to be able to support and make a case for it. And if you can, then find a way to evolve it into the design over some period of time. One, one big design change that helped a company do once, it took us three months to slowly but surely convert from the old way to the new way. And the, and the rule was any time you work on something that touches this new way, change it over. So in three months we finally were using the, completely the new way. Any other questions? Yeah, back there. Thank you. Should we also consider providing and load testing when we are doing some refactoring? Because there are sometimes that we can, we're doing sacrifice as the providing and then the load test. Absolutely. If you're worried about going to slowly, if you're worried, if you're writing high performance code and you're concerned that some of the refactoring is slowing things down, I would definitely recommend running a profiler for sure. You may discover that, you know, a lot of the refactoring to do are perfectly acceptable because the compiler technology is already so smart that it inlines all of those extracted things anyway when it compiles. So, but it doesn't hurt. I mean too many programmers who've never, ever used a profiler. And that kind of scares me, you know. It is important to use one. And also sometimes to look at just the memory footprint. How is memory impacted by what I just did? Have a way to visualize memory of the code. Sure. Hi. Do you recommend using static code analysis tool to detect all these code smells? Well, the thing with static code analysis tools I find is that people tend to find the tool they love, the static tool analysis they will have. They install it, they start looking at some really pretty graphs. Everyone is very, very happy about these graphs. And then after about two to three months no one looks at them anymore. So that, that seems to be what I see out there. And if you really do pay attention to the static analysis, it might help you see things. But you kind of have to make it a habit, right? I have no problem with static analysis. It's just, it has to become a habit to help you see problems. Okay. All right. I guess we're out of time. Thank you very much.