 And I have a talk for you this year's talk is about code smells. This is a term invented by this guy. So it's really, we did not plan that I would follow Katrina, but it's the perfect talk to follow her talk. She just mentioned this guy's name. It's one of the last things she said. This is Kent Beck. This guy is Martin Fowler. He's the man who wrote this book, which is the book that teaches us how to do refactoring. They collaborated together on the third chapter of that book. And that's why they want, it's like naming things wins and the name code smells won. And so the reason you know that term today is because of what these guys did back in the 1990s. Now I wrote a book a couple of years ago. Before that I wrote code for 35 years. I went to my desk every day and wrote code. And now I don't do that. I teach instead. I teach classes in object-oriented design. And in my class I have occasion to ask people if they've heard of code smells. And just like I suspect all of you, every one of my classes, they always say, oh yeah, we know what code smells are. And then I ask them to list five and no one can. I can hear you trying, but really most of you cannot. I know that. And so we all think we know about this term, but it doesn't mean I don't like your code and I can't tell you why. That is not what a code smell is. There is some opinion involved, but really it's not just that I disagree with how you wrote your code. The standard is higher. These smells, these 22 different smells all have very precise meanings. And the power, the magic, if you will, of this list is that they've given things names. Once you give a complex idea name, if we could just all learn what that name stood for, it means that we can talk to each other in an unambiguous way without having miscommunications. And so one of the things they talk about, very often when people talk about code smells, they prefix code smell with the word bad. They say bad smells. But really the definition of a code smell is that it might indicate a problem. Not that it does, that it might. And so you don't have an obligation to filter out all the smells. It's actually important that you not. And it's also worth getting from me with code smells because they have such great names. Okay? Look at this list. Feature envy, that's cool. I like this one, the one that needs a code of conduct. Interpropriate MSC. Shotgun surgery, that's another favorite. Okay? And so the problem with this list, well, first of all, okay, I'll confess, I had that book for a really long time before I read it. I had a really hard time with it, right? It's one of those books that's like a recipe book. And I've heard it described as a cookbook for people. It would be like reading a cookbook if you did not eat. And so I really need the story arc and I had a very hard time following along, like persisting with a bunch of recipes that were just like shrub, shrub, shrub, shrub, shrub. All right? But it turns out it was really worthwhile when I finally did it and actually Katrina made me. I'd say Katrina made me. So there's 22 things on this list. I had a stack overflow at 22. It turns out there's a guy named, it's Mantaya, it'll be in the credits. He wrote a paper where he grouped them. We'll throw those two away. He grouped them in five different categories. And through the magic of keynote, I can do that. This is the only reason we make talks so we can use the effects. And so here, let's just talk about this set, okay? This is a list of things that just do not need to be that big. Long method and large class are probably self-explanatory. Data clump is when you have two or more pieces of data that always appear together. You pass them around over and over again together. Long parameter list is obvious. It might only occur once, but if it's long enough, there's probably an object in there somewhere. And this thing, this other wonderfully named thing called primitive obsession. We saw an example of that in the talk Katrina just gave. This is when you have like an instance of a base class like a string or a number or a hash or an array. And you pass it around to a bunch of objects and they look at it and decide what to do based on something they know about it. So you say, oh, I got a number. It's six, so I'll do thing X or it's eight or I'll do something else. If only the thing you got was smarter, you could just send it messages. So primitive obsession is when the objects are too dumb that you're passing around. These things are grouped in a category called bloaters. They make code bigger than it needs to be in the places where you use them. The next group, this one, these are things that, these are ideas that are available in object-oriented programming that you can misuse. Switch statements, you know they're conditionals by normal people talk, that's conditionals. Refuse bequest is when it's an inheritance problem. You have a subclass that overrides a method that it inherits from a superclass and like throws an exception and says, I don't implement that thing. I refuse the bequest. Alternative classes with different interfaces is pretty obvious. The other thing is temporary field. The thing that temporary field is on this list, right? Temporary fields can be really handy, but sometimes they mean that you should have made a method with that name, right? Why are you giving it a name? What is it about the code that you have now that feels like it needs that name? And so these things are all grouped in a category that he called tool abusers. I have a lot of bikes. I work on bikes. I have a garage full of bikes. And I'm an amateur mechanic, which means I have amateur tools, which sometimes involves a very short wrench, a pipe, and a hammer. All right? And I can tell you, it almost always turns out badly if you abuse your tools. All right, next. This group, this is stuff that makes change hard. So Divergent Change Shotgun Surgery, which we've talked about parallel inheritance hierarchies, which is pretty obvious. Sometimes you have a couple of hierarchies that each have two sides, and then every time you change something, you've got to go add or move these are the kind of things that keep you from wanting to change code or make code hard to change. Now, notice that almost everything I'm talking about, if nothing ever changes, it's probably okay. Like code that's really embarrassing code, it's fine to keep really embarrassing code. You should be brave about your ugly embarrassing code because it is not costing you money if it's not changing. And so just because the smells exist, you know, sometimes you should, like, own them, be proud, walk away. Don't let people make fun of you about having bad code. This next category, lazy class speculative generality. It's okay. On top of the effects, finding the pictures is also fun. Classes that don't do enough is a lazy class, right? It doesn't justify its existence. I'm going to skip speculative generality for a minute. Data class, you know, we are object-oriented programmers. Classes ought to have data and behavior. Duplicated code is pretty obvious. Let me go back to speculative generality. I'm going to ask you to raise your hands again. Who in here has ever written some code for a feature you thought might arrive in the future? Keep your hands up for a minute. All right, I'm going to out myself with you here. Who in here has ever, after many months of working around that code, ripped it out and thrown it away? We are bad guessers. I love object-oriented design. It's a thing that really interests me. This thing of speculative generality where we say, I'm going to do something really cool in my code for some feature that I think we might need later. This is why people say bad things about OO. This is what they blame us for. It's primarily things in that category. You have to be right. The few times that you are right have to really be big wins that way, the enormous cost of being wrong. Code is read many more times than it is written. The cost that we... The reason that we cost money is the time we spent reading code. And if you add generality, you increase the level of abstraction of code. Very often that means adding levels of indirection, which humans are terrible at, but anytime someone looks at that code, it's hard to understand. So we should really try to restrain ourselves and not speculate about the future. When the new requirements come in, they'll tell us how we wish we'd written the code and we can do it then. Dispensibles, sorry, here. Ah, see, I did that thing with a clicker. Dispensibles, you've all seen it now. Okay, so we'll move on. So the last category here is this group. Feature envy and appropriate intimacy, message change in middleman. I have an object. There's Joe's down here. Joe's an object that I know about. And I send him way more messages than I send myself. Could be that I'm more tightly coupled to Joe than I know, right? Inappropriate intimacy would be if Joe had a bunch of private methods and I reached in and got them. Okay, that would be bad. I really am... I really am sort of pushing the code of conduct, aren't I? Sorry. I hope no one is made uncomfortable by that. Message change. There's the law of demeanor. Those violations are right. You got dots. You send a message to something you know about and you get a response back and you send another message to that. If the types change across those dot change, that's a message change. Middleman is if you have an object that you send messages and it's soul, person, and life is to forward those messages to somebody else. Maybe you don't need that object. These things are called... They're grouped together in a group called couplers because the effect of this is that it binds the objects together so that even if they're beautiful, even if you've tested them, even if they're little works of art, you can't ever reach in and get one out and use it in another context. They come as a bundle, all or nothing. And so there you go. 10 minutes and 34 seconds, everything you need to know about code smells. Okay, but we're not done. We're not done. Because now I'm going to talk about refactoring, but not very much because you just heard about it. But there's a thing here that people don't know that those guys discovered in the 90s that I want you to go away today understanding, and it's this. Refactorings, just like code smells have names and they mean very specific things, refactoring have names. Refactorings have names that are very specific and they come with recipes, not hand-wavy recipes, very, very specific concrete recipes. Here's one. This is page 149, Martin Fowler's book. And it looks like a recipe, right? It has numbers down the side. There's little optional clauses here for situations that might be different in your case. You notice that it refers to other recipes here where the things are in capital letters. That's another whole recipe by itself. It's recipes within recipes within recipes. So all of the refactoring is working the same way. This is not something, we don't just wave our hands and say go refactor. Refactoring has a very specific definition. It's to rearrange code without changing its behavior. And all the ways in which you can rearrange code are already written down with instructions by people who really thought a lot about this. And so now you know that code smells have names and they're real things. And refactoring have names and they're real things and they come with recipes. I can give you one last bit of news. Every code smell maps to the curative refactoring recipe. Ponder that for a minute. What does that mean? Here's a cheat sheet. This one is provided by the guys at Industrial Logic. Notice on the top, the code smell they're talking about is data clump. This is a little tiny definition of data clump. That F81 is a reference to page 81 in Martin Fowler's book. The three things on the bottom are the refactoring recipes that are curative for that code smell. So this slide, I just blew it up so you could see it. I extracted it from this PDF, which is a couple pages long. It cross-references all the code smells and refactoring is in Martin Fowler's book. The refactoring book and also a book by another guy named Joshua Karajewski called Refactoring to Patterns. And so it turns out that this is all you need to know. The problem is solved. You do not have to reinvent this wheel. And so it turns out if you just learn, all you need to know is a few things. And many of you, at least my experience, the reason I wanted to give this talk, as my experience from teaching is, somehow I think a generation has passed since all these books were written and that people who are new to programming in the last 15 years, I'm a woman of a certain age, you can tell, right? If you're new within the last 15 years, you may not have this information. I'm going to show you some code now. I'm going to use the last, I don't know, another tool to spend about 15 or 20 minutes looking at code. And I'll show you the practical effect of recognizing code smells in your code and doing refactorings. My class sale is a subclass of persistence. You can think of that as active record. If you're in the back, it will not hurt my feelings if you get up and come forward. That's the font size of my code. Now I was going to be in the keynote room. It just happened. So we have that. And let's say I have my class foo, it has a sales total method, takes some prams, maybe this is a controller like thing or something that a controller calls. It knows the name of the sale class and it knows some other things, right? It knows that sale understands where and it knows that the thing that comes back as a response from sending the where message knows some. It also knows two attributes that are in the sale class. It knows the name of date and the name of cost and it knows that the... it can pass a date range as an argument to where and the query will work. Okay, well, that's fine. And then someone else comes in and does this. Later, someone makes bar. And bar's a lot like sale, but they want weekly totals here. It takes prams. If you only need a starting date to do this, so I can calculate ending date right here and then there's another thing here that knows where and sends some to the thing that takes date range. Then later, so this is like Agile, right? People are taking stuff off the backlog. This is what happens when you don't have code ownership and don't have oversight. I love Agile. I'm not saying Agile is bad, but I see a lot of code that looks like this because people solve the proximate problem and no one has a big picture. So here comes... Oh, yeah, of course, now I have expenses. If I had sales, that's going to happen. Then we got Baz and it does expense total, takes prams, it does this. And then this method blows up because someone passes in a date, a bad date, bad string for dates. Kaboom. And so someone comes in here, they get that bug report and they come in here and they fix it. So now I got this code. Let's just stop a minute and try to identify all of the ideas in this one slide of very simple code. There's the idea that I can calculate a week. I know that. If I pass me bad dates and I can set defaults, there's the notion that start and end date go together as a group always when I need it. There's the idea that I have... I know messages that I can send away to sale on that other object. I know the name of that attribute and I know the name of this attribute. So I got all this stuff. It's marked up a lot of different ways here. And it's still not so bad, right? If nothing ever changes, I don't know, maybe you should walk away. I think that's the most important thing in my life. When I go out in the wild, I spend time at businesses very often that have been successful. So that means they're 5 to 6 or 7 years old and they have enormous code bases that they hate. That's why they call me. That's where I go. You can imagine, right? So if your code base is at home, feel big and embarrass you, it is probably because your business accumulates and so here what happens in these situations is after you have a foo in a barn of baths and someone comes and makes a lary and a curly and a moe and they do kind of the same thing, right? And then someone does a fee and a fine and a foe and then your code looks like this. And eventually, through the power of Keynote, you have duplication and message changes and you have to do shotgun surgery and if you get that wrong, something breaks in production and all the defaults are different in different places and you find yourself unable to understand this code. It feels like the accumulation of simple problems has created an overwhelming mess and it feels like you need to understand everything in order to fix anything. And I can tell you from personal experience that if you try to attack all the problems at once you will never finish. It is not possible to clean up the whole mess at one time. You can't tell them to shut the doors. It is a big two month refactoring. It will be fine. That goes up really well with the people that pay the bills. So what we need is a way to reach in here and somehow see, right? We need a way to pull one problem out and fix it in confidence that it will work and that is where code smells come in. We are going to do data clump. We are going to find the data clump and we are going to fix it with the extract class. It is going to remove those three colors. Here is the data clump. This behavior does not belong in bar and baths. It belongs in whatever object I should make here. I am going to spare you the refactoring because really you can look it up. And my name sucks, sorry. So here is date range. It takes two arguments. It knows how to set defaults. It has two methods, one that returns a range and one that returns a weak range. This is an object that stands in for the data clump. I could use it in that place. And the way you use it is like this. So in foo, I am about to make this method more complicated, which is why people complain about OO. Let's do it, okay? I am going to get an object that has a temporary variable. That is a temporary variable. That is a code smell. And then I will just plug it in right there. If you run FLOG on this, it got worse. I introduced a whole new class and this method is longer. But watch this now. This one, okay. I can just do it. Now I got this. And the other one. I don't need that anymore. I just do the same thing. Now I got this. And so these three methods, there is the data clump right there. The cool thing about data clumps is that if you take and if you find the data clump, you isolate them, you make objects for them. The behavior will coalesce into the object. If you build it, they will come. And what that means is that you can use date ranges everywhere and get consistent behavior throughout your apps. They are weekly range. All the validations work the same in every place. All the defaults work the same in every place. If you change it, the change goes everywhere. And everyone in your app knows to use this. They would prefer to do this rather than to roll their own in every situation. You guys have tons of data clumps in your app. If the only thing you remember from this talk is to go home and make objects out of them, it's a big win. But let's do more. There's a message chain here. Oh, I hate this message chain. I so hate it. I'm going to have to get back to my notes because I wrote a lot of things down about it. So what... I see this everywhere, right? I am way out in my own objects. Food, bar, and bars. And I have a message chain that reaches into my persistent object messages to stuff it gives me back. It's okay for my class to know things about sale, but it's not okay for my class to know things about... things that sale knows about. And it's really hard. I find it hard to convince people about how bad this is. And I drew you two pictures, two completely different pictures to try to express the same idea. Here's the first one. This is about the message passing. There's a whole thing over there. And I send where to sale. And then sale inherits from persistence. Active record for you guys. You can fill that in. And so that message chases up the hierarchy chain. And then behind persistence is this whole big enormous cloud of code that somebody else wrote and gave us for free. We love them for it, but up in there somehow there's a list and there's a bunch of other stuff. There's all kinds of things that happen back there. Eventually somehow persistence is a pretty little head about what that message is. And then some comes back. I get this list back. It comes back to sale and it returns it back to foo. And then the next thing foo does is send a message. All right? That's what the message passing looks like. And I'm going to show you this picture in a different way. This is how I think about it. So that's how I think about it when I'm thinking about messages. Here's how I think about it when I think about testing. I'm trying to test foo. Foo has collaborators. These are the immediate things. I can put a ring around these objects. When I'm writing the unit test for foo, I have to get instances of those collaborators and set them up in order to make my test run. Right? That's how I set up different contexts. However, here in this case, foo knows about that. Something that's not a not a direct collaborator. It's the next ring out. It's in the next circle. That's where the dot is. Every dot jumps a ring here. And so what I do, what's happening here is I know about my collaborators, collaborators. And if you do this, you've probably seen the effects of this in your code. If you don't want to run the database query here, you have to create, you have to stub and stubs or mock and mocks. Or in any situation where there's a message change, sometimes you get in this situation where you're trying to set up a test and you have a million objects, right? You have to instantiate a bunch of things. It's this kind of message change thing that's the problem. If you do that, you're going to hate yourself. And I'm going to go a little bit more to testing about this in a minute, but first I want to just fix the message change. It's really easy to fix this. So I got that problem. All these things they look just about alike. The problem here is that there's a concept that doesn't have a name. That's what happened. I have this duplication. And I haven't given it a name. Now, one of the things that happens a lot of times when you see this is that there's a method name with a repeating prefix or suffix. And when you see that, what you know almost inevitably, you have an object that's crying to get out. And so what this means is that I can hide the delegate by putting a total method on sale. It's as simple as that, right? I'm going to write my own method, and you would probably use a name scope for this. I'm going to write my own method. And then I'm just going to change this to send total. And it's going to happen on expense also, of course. So once I've done that, I end up with this code. And there's no law of demeter violation here. I don't have a message chain up there. And so we fixed it here. We've hidden the delegate. Now, I want to talk a little bit. Justin Searle, who's here somewhere, bless him, gave a talk a couple of years ago. I don't know how long about how much reality you want to use. I can recommend it to you if you haven't seen it. It's a couple of years old now. He can probably tell us what year. 2013. Look up Searle and what's the name of it? He doesn't even... Budgeting reality. So let's look at a kind of test you could write for this code. You could do... Well, okay. So I know I'm sending total to sale. Foo is really mine. Sort of mine. It made a devil's bargain when it inherited off persistence for the benefits it would get to pay the price of that dependency. I'm happy with that bargain. Here's a test. I can new up a new Foo. I can send a message. I can check the output that comes back. This runs the database query. I'm a long way from the database. Foo is not a subclass of active record of persistence, but it's still ran an entire query at this point. And if you do this in enough of your code, some of you have probably already had the experience. I'm going to look at your faces and see if I can tell who. You've had the experience of your test suite running more and more and more slowly as you get more and more and more code. And then finally here's what happens. It falls down. People quit using it. If you want to refactor, like Katrina was showing you, you must have unit tests that run quickly. And if you couple the unit tests for these objects that are far, if you couple the unit tests for your objects to behavior that you don't know that's far away, that's very slow, you're going to be unable to refactor with green as the wall at your back. And so it's really, really, really important. We want your tests for each to run, but even more than that, I want them to run quickly. Okay. Let me just go on. Okay. So the problem here is I'm glued to this class. I'm glued to sale. I know the name of this constant inside my method. And I could instead depend only on the name of the method, the message I want to send. I could do that by injecting a model. Now, I am not unaware of the dispute that we sometimes have about dependency ejection. But I think it's great, and I'm going to try to convince you that you should be using it more. If I do that, then I can now do this. I don't really think I'm not very attached to sale as a class name. I don't really care. I think I'm talking to a totalizable. And if I decide that I'm going to do real OO and think of objects in terms of the roles that they play instead of the class they are, what it means is I'm very comfortable with this idea of decoupling myself from constants and injecting players of roles. If I do that, I can make another player of that role that in my mind it is equally valid. It is just as good. All things, say I polymorphize, everything that implements this message is equally good. I'm just going to make that a little simpler. What I can do is get an instance of that model and inject it in my test. And then I have to assert that the right answer comes back. This is fast. It's really fast. Now, what does it prove? I know people, many people are uncomfortable with this. So let's talk about what this, what it means to do this test. This test does, this test proves that my method, if it's injected with a total, totalizable, someone who implements the total method, will call that method and return the result it gets back. That's what got proved here. What it does not prove is that database queries and Rails work. It does not do that. But let me ask you, the Rails framework has thorough and exhaustive tests for whether or not their database queries work. In addition, my cell class, the cell class is a subclass of persistence. It may be that I have a test on total over there that actually runs a query. But it is not necessary for food to jump across this message chain and do database query, jump across the network and do database queries at the other end. I see when I travel around, slow running tests are killing people, and I think the tests that I see run slow for two main problems. The first one is this message chain. If you have message change in your code, if you commit demater violations, it means that you have to new up the whole network of objects and run all those messages when you run your tests. And that's going to be slow. It's going to be hard to understand and hard to maintain. So you should hide the delegates. You should hide the message change with delegates. The next thing is even after you've done it, which we did here, it's possible that you are tightly coupled to something that's really slow. It's really slow. And if you tightly couple yourself to slow things and don't give yourself a way to substitute other faster players of those roles, you are doomed to slow test. Now, I realize it is possible here to stub in sale, right? I could stub that method, but I and it does work, but I prefer this and it's for... I have a reason, right? It's not just that I like this code better. It's that this makes you think of those objects as players of the role. Not as instances of their class. I can use this totalizable double any place in my test where I want. I could use it over in the expense test, too. It's a really different... It is not the same thing to inject this object and break the coupling. Stubbing in your test is not equivalent to injecting the object and breaking the stubbing. This is a very much more powerful idea that will improve all of your code, not just make that one test faster. So there's one more thing. Let me see how I'm doing on time. Great. I'm going to do one more code refactoring before I finish. Once I've done that, now that I'm here, I have the duplication here. There's a thing called pull up method. Pull it up into the super class. I'm not going to break open active record and monkey patch it with total. It seems like a bad idea, doesn't it? I'm going to put that right there. Notice what I've done. The code that I put in the module does not look exactly like the code that I am promoting from these subclasses. I added some indirection. I injected... This is just like what I just did with model equals sale in the last example. The things that... I had concrete dependencies on my class. I am now injecting as variables and setting defaults. That's my main idea. This gives you flexibility without adding much complexity. Now, there's a name for this code smell. It's called speculative generality. I just did it. I know I admit it. But here's the thing. If the methods are short so that what we want... The problem with speculative generality is you're wrong and it costs you money because it increases the complexity of code in the meantime when you try to read it. When I start to you that at least for me, I know it's partly what you're used to. For me, my methods are short and I routinely inject the dependencies. And so I do not find... I find that the increase in flexibility is more than outweighs the increase in complexity. This does not feel very much more complex to me. It's way more useful. Now, anybody can call this method on a date with... on any attribute and some any value. I would use a module. Okay, wait. I just put that in this morning which is why I didn't know it was on the slide. Dependency injection is awesome and you should be using it. So we can use the module just like there. So there we go. And so that's it. If you're not intimately familiar with code smells, you can move yourself... If you're waiting to move up to that next level of programming to make that transition to the next level of proficiency, the way to do it is to go learn code smells and to learn those recipes for refactoring. These ideas are not new. They've been around... That book was written in the 1990s and I know that many of you here probably began programming since then. Like for some of you, this book is older than you are. And so you might have the sense that it is part from the Old Testament of programming. Right? But it's a body of work that is practical concrete advice about how to write code. If you're aware of it, you should go learn it. Many problems that we have that feel insurmountable can be solved by purely mechanical operations. And the cool thing is it doesn't take the fun out of writing code. It's not like there's nothing left for you to do. What it does is it removes the tedium safely so that you can concentrate on the harder problems. So if you learn... When you learn code smells, it's like putting on glasses. Right? It turns that mess into something like this. Following the smells lets you make straightforward, easy-to-understand objects that are easy to test and a pleasure to reuse. Wait for it. So, while you're learning code smells, you should look at Reek. Is Peter here? Maybe not. Okay, this is the... Notice their wonderful icon. Is that cool or what? So Reek is a static analysis tool that you can run against your code so you don't even need to really go figure it out. You can just run the damn tool on your... Sorry, I have a policy never to do that from stage. You can run this tool on your code and it will tell you what to go look at. So smells aren't a bad thing. They're actually great and they're all over our code but you have to kind of lean into them. Like, okay, I have a new puppy and if that were my puppy he would reach in and bite off one of those petals. And you should do it too, right? It's time to learn more about smells. They're just information. And much of the information is neutral. If you're trying to lose weight, maybe you should avoid pizza. If you've been on a long bike ride, it's the best smell ever. So when I look back at my old code, code I wrote three years or four years or five years ago, it's really common for me to look at the code and say, what fool wrote this? It's a wonderful thing about our jobs. It's the very best thing, right? What job can you imagine having? Can you imagine having a job that you were no better today than you were five years ago? It's such a depressing thought. I love looking at my old code and hating it because it means I'm better. It means I've learned something since then and what a wonderful thing that is. So I started this talk with a cup of coffee and I'm going to end the same way, right? It's late. I know you're tired. It's probably not the best time to go drink coffee right now. But tomorrow morning when you get up, it's going to be a great smell. I hope you're getting. Thank you.