 First thing I just kind of want to get out of the way real quick is make sure that we all have a common understanding of what refactoring is, particularly in the context of legacy code. So if you were in Arlo's session earlier, he talked a little bit about this, but refactoring really is changing the design and structure, perhaps, of existing code without altering its behavior. Okay? So it's not fixing bugs. It's not adding new features. It's not doing a whole, you know, just writing test cases is not refactoring. It's strictly about not adding new behavior, not changing the behavior of a system, okay? And improving the design in the meantime. And really, the only time that you would really want to even refactor legacy code is if you were planning to change it. So if it's already out there in production, running, and working, there's really no reason to go in and refactor it. It doesn't really make much sense. Okay. So hopefully this doesn't keep happening. All right. So this talk is going to be a, it's going to be dealing with a Java code base and it's got me doing some code, which is all part of this fabulous movie that I put together. All right. So there's four rules of simple design. The first rule is that it passes all the tests. Second is that it reveals intent. Third, don't repeat yourself. And fourth is fewer parts. We'll be going through each one of these in the context of the refactoring. There's also one rule of legacy code refactoring, which is don't break it. So don't break it. You got to make sure everything's working. All right. So let's move on to the code base. So this is the overview of the code that we're going to be looking at. This is taken from a production system. I've stripped out some of the miscellaneous, I've stripped out this portion of the code to make it consumable in this session. So first we have this custom job execution listener. Hopefully you can read this okay from back there. And that's a, it's like a spring-based application, so that's a spring class. And there's a date time util, which as you might expect has date time utilities. And that's a fairly large class. We have a file util, which anybody know what that does? File utilities, that's right. Okay, so this is actually quite a really huge class. You can see here's the little scroll bar. So it's got, I think somewhere around 500,000 lines of code, so something like that. And we have one last file which is the infamous constants file, which any good project, any good legacy project at least has one of these. So this is a constant file where you throw all of the constants that are used anywhere in your application. All right, so this is our legacy code base we're going to be dealing with. We'll start with rule four of simple design, which is fewer parts. One of the first things I love to do when I'm tackling legacy code bases is look for dead code. So in most IDEs, this is Eclipse, there are settings for it to be able to help you find dead code. So it's got things like our local variables used, our parameters used, unused type parameters, imports, et cetera. So I generally go and I'll make sure I enable all of these things. And as a side note, all these settings that you make for your particular project, in my opinion, should then be shared with the team, whether via config files or however it's done, so that everybody's using the same set of configurations. So you can see that this has notified us that there are 10 things that are not used. It ranges from import statements to methods that are not used. So the next step is going to be eliminate the dead code. So here we have, these are just kind of basic things. Import, let's make sure we get rid of things like that. And we have, this is a method that is not used locally. So Eclipse knows because it's a private method that's only used in this class, nobody's calling it, so we can just get rid of it. There's multiple ways to do it in Eclipse. You can either hit Command 1 or Control 1 if you're on Windows. And that's like a little quick fix or you can click on the little thing on the sidebar and do remove method. So there's various different ways to do it. And you can also highlight multiple, you can select multiple problems and then fix them all at once, i.e. remove them all at once. Okay, so we're removing some local variables. Here's a local variable int i equals zero. And then inside this little loop, there's a i plus plus, so we're incrementing that counter. But it's not actually being used anywhere. So it's values being written but never read. That's an unnecessary statement. And actually the JVM probably is gonna be taking that out during at runtime anyway, so we can get rid of that. And then the last one that we have here is the value of the secondary. The value of the parameter is not used. So we've got a parameter here, this secondary file, which is used nowhere in this method. You can't really see it, but it's highlighted in yellow there. And you can see right above that is where it's being called in this GetMain, Ctax, blah, blah, blah, blah. The second parameter is just being passed in as a empty string. So Eclipse has this nice feature, change method signature, command option C. Learn the shortcuts, those are great shortcuts to use. If you hit that, you'll get this little pop up and you'll get a list of your parameters. You've got your input file name, secondary file name. We'll take that secondary file, click Remove, and you can notice that when it changes it, not only will the function be changed, but the calling location that parameter will be eliminated as well. So that's a nice little refactoring technique. And I think that's the last one here. Okay, so that clears out all of the unused code, dead code that Eclipse knows about. Okay, so I also like to make changes in very small little chunks. So after every little change like this, we'll commit. So get check in, okay. Okay, so next, find more dead code. There's always more dead code. That's just the beginning, okay. So there's a great plugin for Eclipse and I believe this is available for other tools as well called UC Detector. Highly recommended. It will help you find things that Eclipse doesn't know the first thing about. So you can install that and then we use it as you just right click on the project and do the UC Detector, Detect Unnecessary Code. And what it'll do is it'll go through and it will find things like public methods and public fields that are not used anywhere in the code that's only invoked by test methods, etc. You have to be a little bit careful because if you have public methods, you have to make sure that those aren't being used by an outside API or something like that. And like this one here, this says class custom job execution listener has zero references. Well, that's because it's the entry point for the code. So we're not gonna obviously delete the entry point for the code. So you have to pay a little more attention to what's going on here. But there are certainly things that, so this public constant here, it's just unused. Nobody in the code base is using it. We know this is not a public API. This is a internally used system. So we can start deleting some of this stuff. So this is probably my, this is like the best. Do you guys like deleting code, anyone? It's like so good. I could watch somebody delete code all day. I consider it kind of a net positive day if at the end of my day, my net contribution has been negative lines of code. And it's not really a joke, it sounds funny, but it actually is true. Because even when you're just doing any other types of refactoring, oftentimes you're finding abstractions and things and removing duplication and all that kind of thing. So it can be a really nice thing. So here again, there's a whole bunch of methods that aren't being used. We can just select them all at once and delete import statements. And once you have a code base that's kind of under control like this and doesn't have all of these things, you can actually set up your editors to automatically fix the imports and automatically flag you and let you know when you're introducing unused fields and so on. So there's another one, okay? And oftentimes what you'll see happening is as you start deleting code, more dead code will start appearing because the code that, there's now additional code that was being used only by the things that you had just deleted. So you kind of have to keep going in this cycle until you've hit everything. So I believe that's what I'm doing here, so a little bit of that. And unfortunately this particular tool, this code detector, UC code detector, leaves a bunch of white space in your code. So you just reformat it and it fixes it. But that takes care of that, okay? Some of these are flagged because it's suggesting that you should change them to package level rather than public. I tend not to pay too much attention to those, but it just depends on what your tolerance is. Okay, one other field? Okay, I think that's the last one. All right, all right, make sure we check in. First time I did this, it was live and it took way longer, because I didn't have the ability to fast forward myself. So that was problematic, okay? All right, come on, type faster. Okay, our next step, we're gonna detect unnecessary code again. So this is just going to go through that same thing. Just kind of what I was talking about. You have these methods that are now, since the other code has gone, now no longer have any references. So go through and get rid of those. This should happen pretty quickly, I think, delete code. So in this one, what I've basically been doing here is highlighting them, hitting command one or control one, and then hitting okay, okay, delete, delete, delete. So in this whole presentation, it is compressed down into about 43 minutes or so, and the actual time that it took me to do all of this refactoring was maybe on the order of, I don't know, like an hour and a half or two hours. So it's not, it doesn't really take that much time, so it's worth doing. And again, remember that we are not changing the behavior, so all of these things that we're doing are completely safe. That was me overzealously deleting. I actually deleted something and introduced a error, compile error, okay. There we go. Okay, reformat it again, and now we're good. So if you remember that Constance file, it used to be really quite enormous, and now it's only a few lines. Check that one in. Very nice comments, very, doing a real good job there, okay. Okay, so the next thing we wanna do is remove unneeded classes. So this is again part of the minimizing moving parts, okay. So we have this class, the Constance class, and it has this field called blank, which is used in the custom job executioner, listener class. We also have these default processing file path and so on. They are also only used in the custom execution listener. We've got the split regex, which is used only in the file util class, and we've got the unknown constant that's only used in the getProster in the file util as well. So what we can do is we can move members and we just type a location for where we wanna move it, and we can move the code, or that constant into the location where it's actually being used. So that way we're kind of getting better encapsulation and things like that. We're not leaking abstractions outside of our classes. So we'll do the same thing with these other constants. It makes you wonder why they were put in a Constance class. Sometimes people put them things there just because they are constants and therefore belong in a Constance class? I don't know. Who knows why it's there? It's there. So we fixed it, and now things are back to where they should be. They're back to where they belong. And now we have this empty Constance interface and we can get rid of it. So hey, hey for us. Okay, we have one less class and we've actually made the code clear in the process of doing that. You can see here, these are the moved variables and probably what I didn't do, but I probably would do is make these private at this point, but that's another little thing that you could do. All right. And these are the ones that we moved to file util. All right, so we'll check in that code. I believe the next check in, I actually fast forward so it'll go faster. Okay. So there's the Constance class checked in. Okay, we're gonna go to rule two now, which is reveal intent. And this is rule two, simple design. We're gonna talk about comments. Everybody love comments? Everyone's favorite thing? Okay. A lot of times comments lie. We have this thing up here that says this resource facade supports out of bound file operations. So let's take a look at this class. Anybody notice anything about a resource facade? I don't even really know what a resource facade is, but I don't see anything about it here. And I don't really know what out of bound file operations, I guess operations are going somewhere, I don't know. It doesn't seem to be adding any value, so we just delete it. These author tags that are rampant throughout Java, we have source control these days to relatively new invention, but it keeps track of who made changes to code, and you can just delete that. We also don't need things that tell us that things are constructors, for example, those types of comments. And I find that a lot of times, these types of comments get into code bases because they, pause it for a second, they get into code bases because people have made settings on their IDE that are enforcing you to put Java.comments and things like that. Get rid of those things. You only need comments if you really, really are having a hard time explaining the intent of your methods or variables. And if you're really having trouble explaining the intent of your methods or variables, maybe you should work on revealing the intent of your methods or variables rather than adding a comment. So with the exception of public APIs. Okay, continue. All right, so in this case, we have a comment here that says this method gets the CTX main file name for the split file name. It has a parameter called name, which is a file name, and it has a return, returns a file name. Not super helpful. Again, this looks like maybe an automatically generated thing or something, but it is not really adding a lot of value. And in addition, it says gets the CTX main file name. This thing says set up file is the name of the method. There's some sort of disconnect there, but those Java doc, the param things we can get rid of, those weren't really adding anything whatsoever. And if that's what this method is supposed to be doing, assuming that it's correct, I don't know if it's correct, but let's assume that it's correct, then it would be better for us to simply rename this to be what the comment says. And this is gonna be arguably a pretty poor name for a method, but it's a better name than set up file. So we're gonna call this instead, retrieve CTX main file name from the split file name. So it's long, but luckily we have a lot of bits these days, so we can have long file names. And this is a good, when we look at this later on, we'll say that looks like a terrible file name. Let me think about what this does in more specificity and we'll probably come up with a better name. But this is better than set up file, okay? So we'll fix that. Okay, so we wanna make sure, so revealing intent, that's what this is all about. So comments are often disguisers of intent and do not really reveal intent, okay? Rename vigorously, unfortunately in this section I don't really rename vigorously, but I'm trying to tell you you should rename vigorously. So renaming is great. So we've got this variable here called ppath and we have another one called procdate. It appears that it's getting the string called processing file path. So why don't we just call the variable processing file path? Again, this whole thing about single character abbreviations and things like that. We have plenty of bits, so let's use them. Processing file path is a good name, or decent name. This next one is procdate. Again, this is process date. Why did we abbreviate it to proc? I don't know, so let's call it process date. And when we do things like this, what we start noticing is that we have one field here called processing file path. The other one is called process date. Why aren't they both processing or why aren't they both processed? There's something weird there. I don't exactly know what it is, but it makes it more obvious that there's some sort of discrepancy and maybe it's something we should look at later on. So that's the extent of rename vigorously. Clearly I should have done more. So renaming is a good, really easy, safe, fast thing to do. All right, so, what was the second call? What's that? Oh, extract method, okay, thank you. All right, so we're extracting methods here. So we have this section of code. This is a very long method. Well, I mean it just depends on your context, I suppose, but in this code base it's a long method. And what I like to do is try to get methods to a minimum of, at least it should fit on a screen. Okay, if it doesn't fit on a screen, it's way too big. So there's this whole section of code in the middle here that's all about getting the log file name. So what we're gonna do is just highlight this whole thing. And this is another great shortcut to remember if you're using whatever ID you use. In Eclipse it's command option M for extract. Okay, so pointing out the comment or the shortcut. We hit that and then all you do is you just type in the new method name what you want it to be called. In this case we're gonna call it setup log file and notice that it has all the parameters that it's gonna need, it'll set it up properly and there'll be no, again a perfectly safe thing to do. All right, so now we have this little log file name equals setup log file and it passes in some parameters. We now can fit our method at least on one page. Still not great, but let's see if we can do a little better. There seems to be this whole section of code right here which is dealing with this file util move. It's doing something about moving the files around. So let's call this method move files. So we did command option M again and away we go. All right, so now we have a section that's moving the files. It's getting a little bit better, a little bit more manageable. We can look at this and say, okay, well if we need to figure out where files move we can go in there and do it. If we need to find out about the file name we can go to the other method and so on. All right, so rule number one is all about passes, all tests. And here we're gonna be talking about characterization tests. So characterization tests are meant to capture the behavior of code. This is not about test driving, this is not about finding bugs or anything like that. Or I'm just moving a method to put it in the right spot because it was out of order. But this is simply about capturing the behavior of existing code. It might reveal to us that there's a bug in it but that is not our concern right now because we're not changing the behavior of the code. So we're gonna write some characterization tests. In order to write characterization tests or tests of any kind it's often the case where you have to change the scope of the method. So in this case I removed the private notation on there. If you were test driving this code obviously you wouldn't need to do that because you would only be testing public methods. All right, so we're gonna create a test class and we have our first test class here using JUnit. First thing we're gonna test is this first line here log file name and there's a condition here where it's testing for the blankness of the file path that was passed in. So let's call this first test. We wanna know what happens if you pass in a blank file name or a blank file pass path, I don't know. All right, so the first thing we do whenever you write a new test, in my opinion you write it and run it. Just run it, make sure it's actually failing. You don't wanna get into a state where you've written a bunch of code and then realize it's passing but you're actually running the wrong test. So verify that it fails so you're running the right thing. Okay, first thing we have is creating this custom job executioner listener instance. This is the method that we're testing and then we have an assertion after that. So all tests generally have three sets, a range, act, and assert. This method takes four parameters so we're gonna pull out some variables for those. In here I'm just using command one to generate the variable declarations. All right. Okay, so we've got a variable here now called logfile and what do we want to know? We want to assert that the logfile has some value and we don't know what the value is. We don't really care what the value is. We wanna run the test to find out what the value is and then we'll just encode that here. So we don't need to worry too much about figuring out what the assert is at this point. There's also this section down here about MDC which is a logging terminology for map diagnostic context, something specific for logging. Don't have to worry about it too much but we're also gonna just write a test for that to make sure that MDC is set properly. Okay, so we'll say MDC and we'll again leave the assertion blank. All right. So what should these parameters going in here be? So we're gonna start out with kind of the simplest thing possible. We'll just put in some dummy values and see what happens. So we'll just put nulls and base cases here. All right, so we got a null pointer exception. Clearly that's not what the code is supposed to do. So let's see where we're getting the null pointer exception and it appears to be that this job execution thing is null. So we need to give that job execution a value and in order to do that, so this is a spring class, this job execution. If anybody's dealt with spring, I love spring as much as the next person but sometimes it's hard to deal with. So a good way to deal with it, particularly on legacy code bases is to create mocks. Anybody not familiar with mocks? Okay, all right, so basically all we're saying here is this is a fake class. We're gonna take this job execution class and just create a fake instance of it that can do our bidding. So what we wanted to do is that when we call this get job instance method, we want it to return us a canned value. So we're gonna say when we call job execution, showing my excellent typing skills, okay, when job execution dot get job instance, there it is, all right. So when I call that method, what I wanted to return is something, what is it supposed to return? It's supposed to return some job parameters or no, it's supposed to return a job instance, there we go. All right, so again, we need a variable of job instance and by the way, if this seems kind of tedious, it is and this is kind of what happens when you're dealing with legacy code. If you were TDDing this code, then this type of thing generally doesn't happen. Okay, so we have a job instance class. Same thing here, we need to call the get job name on that and we want to get a canned value back. So we're gonna create this as a mock as well. We've got a job instance there. We'll set up an expectation here where when we call the job instance get job name, then it will return us some name. And everyone knows the best job name is Fred. So there it is. Okay, so we've got a job name, so hopefully now we're gonna run this test and see what kind of value pops out, which is gonna be some sort of really great log file name. So there we go, let's run it. Okay. All right, so we run it and you can see the expectation down here, it's a little hard to see, but basically it says Fred, 2015, 5, 2015-10-10. Arguably a pretty bad name for a log file, but that's what it is. And again, we're not here to change the behavior of the system and make it a better file name. We just want to capture how it's actually working. All right, and keep in mind that remember, the goal of this refactoring is to make our code ready for modification. So we're getting to the point where we want to be able to modify the code, because doing that without these types of tests can be hazardous. So we've got this, we're testing also now the map diagnostic context because it turns out they're the same value. So how do we know what to test next? A good way to do that is using code coverage on legacy code bases. Eclipse, again, has a plugin called Ekilema that you can install. And Emma is the name of the Java code coverage, Dealey, and it's got plugins for pretty much everything or you can even run it command line. Okay, so that's a great tool, so install that. And then instead of running it normally, you run it through via the code coverage tool and you'll get a little different output. So what it'll show you is highlighted in green, which is probably hard for you to see. Highlighted in green is all the code that's actually been executed. In this case, since the test ran, the test was executed. And highlighted in blue, or sorry, highlighted in yellow are lines that have only been partially executed because this is a ternary operator here, so it's only one of the branches in that line was executed and red means it hasn't been executed at all. So that can be a good indicator as to what types of things we should test next. So let's quickly check this guy in. Got our first test in there. Yay, characterization test. Okay, move on to rule three, which is don't repeat yourself. And the way we're gonna show that is through more characterization tests. So we've done blank file path and as you probably can guess, the next obvious test would be a non-blank file path. So let's just start by copying this test and create a new one here and this will be called non-blank file path. Okay, there we go. Okay, so here we've got the file path being passed in. So instead of passing in a blank string, we'll just pass in something else called blah.text and we'll run this and see what happens. Again, we don't care what the results are, we just wanna capture the behavior. All right, so in this case, apparently if you pass in a file path, it just uses the file name as the log file name. Okay, so we'll put blah.text there and run it again and it looks like the map diagnostic context also is going to be blah.text. So set both of those values. Okay, so now we've got another characterization test and remember that this section is headed under the, don't repeat yourself rule of simple design. So, well, the first thing we can notice here is that this line is now green. Doesn't look green over there, but it is green and so that means that we've covered all the branches on that line. Okay, so now we've got these two tests where we're repeating a fair amount of information and if you went to Gerard's talk this morning, he talked about writing good unit tests and one of the things he was talking about is making sure that you are your test, it's obvious within your test what the important pieces are. So in this right now, there's a whole bunch of stuff in there, we can't really tell what's the significant portion of that test. So we're gonna move some of the common things up to the setup method and there we go, move that. Anything that's duplicated, we'll just move it up there, convert some of these variables to fields. Again, that's another Eclipse quick fix, you just hit command one and say convert to field and it will do it for you, okay? So those guys are common now, looks like the Eclipse is trying to catch up to my rapid editing, okay. This is a very old computer, it's gotta be nice to it. Okay, all right, so now that we've moved some of that stuff into the common method, we can take it out of the second method as well, or the second test and there we go. So we've gotten out some of the common stuff, so now it becomes a little more obvious, it's still not perfect, but we can see, it's a little bit easier to see that okay, the difference between this and this is this line here that says file path, I believe a little later on that I'm gonna move some more stuff out of there as well. Okay, all right, so there are two things, okay. So we also have this assertion here, which is fairly common between these two methods and the only difference, the unique part about it is the actual value, so that Fred 2015 value. So this is a fairly common pattern when you're extracting methods and what you wanna do is find the common, or the unique part of the section that you want to extract and pull that out as a local variable, okay. So first we pull that out like that, so we'll call that the expected file name, because we wanna be able to make something that where we can pass in an expected file name. The next thing to do is select the code that you want to extract, and then using our handy dandy extract method, we just extract it and you can see that any of the parameters that it needed, including this thing here, expected file name are now a parameter. If we had not extracted this expected file name first, then that whole Fred thing would be in, it would have been inside this method, which is not really what we wanted. We wanted that to be a parameter, okay. And then once we've done that, we can then go back to expected file name and inline it back in, because there's really no reason for it to be a separate, a separate variable. So now we have a sort log file and it's just right there. So that's a pretty common pattern, extract the local variables, extract the method and then re-inline, okay. Now we can use that same method in the other, that same assertion in the non-blank file path. Okay, so the don't repeat yourself applies to tests as well as code, as production code. Okay, so there's our little assertion, very nice. Okay, and I think, here we go, we're gonna run our code coverage, or just run it again, make sure it still works. Every time you make a change to a test, it's a good idea to run it. The other thing is, it's also a very nice idea to get a shortcut for your running of tests so that you can just rapidly run them, so you don't have to keep using the menu options and so on. All right, so our next test that we're gonna write is, okay. Okay, this one is gonna be about the agency. So if you saw that line up there, it had a, the job name that was coming in was called about the agency debt extract. So it behaves differently when that agency debt extract has, is the value, okay. So we're gonna go ahead and run this again and see what the file name should be. Okay, that's another compilation error, there we go. All right, so we run this guy and let's see what we get. Okay, so this time it gives us a kind of a cryptic error, unfortunately, it says something about the illegal argument exception, the partial must not be null or something. Basically, this is a problem error in the date time utilities that it's using, but we'll kind of gloss over that and suffice it to say this job parameters getDate thing is returning us a null value and that's what we need to fix. So when we call jobParameters.getDate, we need to make sure that it gives us a good value. And in this case, we're gonna try and set that value. So when we take a look at our job parameters, we wanna set that value in there. And look, there's no set method. So there's no way to set a date. So it looks like this is some sort of a map. So we'll get the parameters, we'll get that map and then maybe we can set a value on that. And it appears that there's no way to set a value or you can put the date, but you need to supply it a parameter, which is a job parameter object. Unfortunately, job parameter object is also this kind of crazy spring batch thing. It's starting to get a little more complicated than we'd like and we don't really, we don't again care about how do you set up job parameters and all that. That's way outside of the scope of this test. So we're just gonna make this into a mock, this job parameters thing and just simplify the whole thing. All right, so we have our job parameters mock now and now we can set a value on it really easily. Here we go. So when we have a job parameters and we call get date on that particular key, then we want it to give us back a, I think it's a Joda time local date or something like that, so okay. And we're having it return us this 2015, 10, 10. If I was doing this again, I probably would give it a different value because we're already using this 2015, 10, 10. So there's possibility of some sort of, that we're not capturing some behavior that we want by using that same value, okay. All right, so we've got that job parameters thing set and we'll run our test again, see if that fixed it. Okay, and it says, okay, so it says the value should be agency debt extract null, 2015, 10, 10. Probably again, not exactly what we meant. There's probably not supposed to be a null in that thing, so let's see where that came from. The null comes from apparently this, again, it's another job parameters thing where we're trying to get this agency ID out of the job parameters. So yeah, so here we go. So I'm not in love with mocks. I don't think they're the greatest thing ever, but this is a great situation to be using them in when we're trying not to modify our design or to modify the production code. Okay, so we made that agency ID return ABC and then that apparently becomes part of the log file name. So now it says agency debt extract ABC, 2015, 10, 10 and we have a passing test. And as it turns out, the MDC is still the same so that assertion extract that we did worked out just fine. Okay, so we run our code coverage again. Now you can see that these first two blocks have actually been covered. Okay, so we've got that first one and then the first if statement also has been covered. And I believe we're gonna do, okay, so now we're gonna talk about refactoring tests as well. So we have in here the file path and process date. Again, these seem to be essentially the same for all of these methods. These file path were not changing, the process date were not changing with the exception of that very first test where we're setting the file path to null. So we can go ahead and move these values up to the setup method, get rid of some of that duplication. And then only in the case of this first one where we have a blank file path we'll leave that essentially setting a new file path locally for that test, but the other ones no longer need it. Okay, so that again makes it a little bit clearer as to what exactly the test is doing. So it went from a fairly long method to it's now just a three line arrange act assert. All right, so again, now before we, our goal here is to be able to change that method or change that method, actually change the behavior of the method. And so in order to do that, we wanna make sure that we get full code coverage on the method. So we're just going through here and essentially doing the same type of tests for all of these things, the same process, checking the output of the test and then changing the assertion to match whatever the output came to be. We'll go through that for all of these. Should just take another minute here. And the other thing to recognize when you're writing these characterization tests is that while it will give you a good degree of confidence that you haven't broken something when you actually go to change the behavior, it's not a guarantee because you're not testing every combination of everything. You're not testing possibly like all the error conditions that could come up. All bunch of things that we're not testing. So that's just something to keep in mind that it's not 100% reliable but you are gonna get pretty darn close and you're gonna be in a lot better state than you would be if you had no tests around it. Significantly better state. And okay, so we'll run this. I think we're at the point where we have all of the code covered. Okay, we've got all our tests, eight tests. And it looks like, there it is. So okay, so all of the code has been covered. Okay, so now we're in a place where we are ready to actually make a change to this code if we feel like it. And so now we're gonna safely refactor. So we'll do just do a couple of small refactorings here. The first thing is that we can notice that, okay, apparently I felt the need to run the test again. Okay, so we can see that we've got this not here. That's kind of weird. So we just invert that so that we start with a non-negative condition. It just makes things easier to read rather than have to read not. You can just check if it's blank. It makes a little more sense. We've got all of these if statements have this logging thing at the end. So let's take that out and we'll put it at the bottom. Now this is a behavioral change because there is one condition where, if it didn't meet any of these conditions, it actually will still log in this case whereas before our change, it would not log that if none of those conditions match. So we have actually introduced a behavior change now but the important stuff about the MDC and the log file name we've already captured. So hopefully that behavior will not get ruined. We're also gonna now extract this, that little block of code which is all about specifically generating that file name. So we're gonna pull that out into a smaller method. So now we have this little setup log file thing now is fairly easy. We're just getting the file name and then setting up the diagnostic context and that's pretty much it. Very straightforward. All right, so this generate log file name is kind of weird. We've got a bunch of if conditions. Obviously only one of these things is ever gonna match. It's checking the same variable every time. There's really no need for it to do that. So let's convert that to be if else's instead. Save us some execution paths. And then if you look at the very first line, I think that's the next thing I've been doing here. Okay, so we'll run it. Make sure that still works. All of our values are still the same. This log file name, it's being set at the very beginning and then it's being potentially overridden. Essentially what that means is this is the default value. So we're gonna move that to the end and we just put that in the last else statement. Makes it a little bit easier to read. And then also now that it's in this else statement, we have all these if else's and then we have a ternary operator in the middle of an else. So that's kind of weird. So we'll break that back out into a regular if else. So again, Eclipse has little things that'll do that for you. It'll replace a ternary with if else. So we can do that. And so the changes like that to me feel more scary than the rename refactors and things like that. So I always wanna make sure that I have really good coverage before doing things like this. All right, so there's that. And then we've got, I believe just two small changes left. I was just gonna tell him you should run the tests. Good thing. Okay, so you can see here now we've got this log file variable, a log file name, we're setting that everywhere and then returning it at the end. I'm just gonna replace that with just a return everywhere. Some people object to this kind of thing because you're interrupting the control flow. I think it's fine as long as you have short methods. When you have really long methods, this is problematic. I'm out of time but I literally have half a minute left so I'm gonna keep going. And the other thing that you get out of this is that you now have an immutable variable. You don't have that variable that you keep changing log file over and over again. And immutability is probably a good topic for another talk but it's a good thing, suffice it to say. So we have that. And then I believe the last thing that we're gonna change here, you can see there's all sorts of duplication here, this job execution, get job instance, get job name. That whole thing is just repeated over and over again so we're just gonna extract that out into a local variable and it will replace everywhere at once. And I think that was the last change I'm gonna make for now. But the idea then is that you can now get in there and at least with this, that method change fairly willy-nilly and it should refactor it in non IDE ways that you may not expect or may not know whether with 100% confidence that they're safe but you can now do it with some confidence. And I think I'm just gonna check in here. That would be a good idea. All right, so that's it. I'm checking those guys in and that's the end of that part. So I think I still have like probably a minute for... One minute? One minute, all right. So one minute for any questions or I'll let you go a minute early, yeah. I'm also a big fan of deleting codes but the way we did it without any kind of test first that leaves me a bit uncomfortable. So deleting the code without the test made you feel uncomfortable. So it depends on what you're doing. So in the case where there are private methods that are not invoked anywhere. It doesn't matter. There's no test for it. So I mean, what are you gonna test? You're gonna test that you call it and your test would call it but nobody else calls it. So I mean, I'm not sure what you would do. So or how you would test that. So those are okay to delete. Same with anything private that's not used anywhere else is fine to delete. And again, as long as you're not dealing with public APIs or anything that's publicly exposed outside of your universe and you have control over all of it, then you can delete. So it does seem a little scary at first but it's not, I don't think. All right, and I think that's my official end time. I'll be around here for a little while and then catch me anytime later if you wanna chat. I'll be here all day tomorrow. All right, thanks very much. Thank you.