How to Avoid Refactoring Legacy Code HELL

have you ever been in a situation where you had to work on Legacy code and as soon as you took a look at it you basically want to jump out of a window well I have but today I'm gonna teach you a framework that consists of five steps and if you follow this framework working on Legacy code is going to be way easier for you from now on and to show you how it works I'm going to do the so-called gilded Rose cutter and this Kata was originally created by Terry Hughes it's really fun if you want to try this yourself there's a link to the git repository with the counter in different programming languages and description of the video now let's dive in let's start by looking at the requirements of this Kata so it's about an in called the Gilded Rose and of course any respectable end is going to run custom python script so that's what we're going to look at today basically the idea is they buy and they sell Goods unfortunately these items decrease in quality as the sell by date nears the starting point is a python script it's called gildedrose.pi that contains one function called update quality and that gets a list of items I made a few minor is here that I added a type annotation so this gets a number of items and that comes from an item file that's what you see here item is basically nothing else than a container with a name a cell in and a quality value and to make things a bit easy to work with the only thing I did was added these type annotations and that's it basically now officially according to the requirements I wasn't allowed to change the item class but I still did it just so that I have my type annotations and fairly it was still allowed to make a few superficial changes so back to requirements so all of these items have a sell in value and that's the number of days in which the item needs to be sold all items also have a quality value that indicates how valuable the item is what the quality is and at the end of each day the system lowers the value for each item so that as time progresses the sell-in date goes to zero and the quality drops but there is a couple of special cases of course for example once the sell-by date has passed and the quality degrades twice as fast of course there are some other things like quality can never be negative and there's also for specific types of items specific types of behavior like agent increases in quality the older it gets also quality is never more than 50. sulfurus is a legendary item that never has to be sold and it doesn't decrease in quality backstage passes just like a speed increases in quality as the cell and value approaches and then there's some other custom rules that the quality increased by two when there are 10 days or less by three when there are five days or less but it drops to zero after the concert the aim of the card is to add a new type of item which is often something that you need to do right you start with old code and then you need to extend it at a new type of thing so that's the countdown and like I said the starting point of this Kata is this single update Quality Function that has all of these different if else statements and complicated logic that we're going to have to refactor I'm going to refactor this code and then after I've done that I'm going to cover the framework that I followed for this but before I start changing anything let's first analyze the code a bit and see what's going on so overall like the requirements say and we also see this in the item class that each item has a sell in value and a quality value and there's also a name and these things are updated every day by simply calling this update Quality Function we also see that there's default Behavior so this is kind of the default behavior of items right selling days decreases quality decreases but then for special items there is specific kind of behavior so that looks like a need for some sort of inheritance relationship to override Behavior but of course currently if you look at the code you see that as one huge for Loop which contains all of these different if else statements there's also lots of magic values in here like these item names that are duplicated several times and it's clear that this is code that's incredibly hard to work on because you know if we wanted to add a new item type I have no idea where to start basically because everything is like mixed up and it's completely unclear what will happen if I modify this because there are so many special cases and edge cases that we have to think about for example if I start modifying the code here it's impossible for me to understand how this is going to affect the entire system and if all the edge cases are still handled properly so it's a pain to work on for sure by the way if you want to get better at understanding code detecting patterns reviewing code you should definitely do my free code diagnosis Workshop the link is below this teaches you to review code efficiently while still detecting the major problems it shows you how to do that by looking at actual production code libraries that you probably are using in your own projects as well so to enroll into the workshop go to iron.cole slash diagnosis the link is also in description of this video now back to the refactoring our goal is to refactor the code so that in the future it's going to be easier to add new types of items and that we can be confident it's not going to break anything else in the system and of course the way we can measure that at the end of the refactoring is by actually adding a new item and verify that it's easy to do now especially when you're refactoring Legacy code like this one thing that's really important to do is write tests because tests are basically your safety net and fortunately well fortunately there is actually already a test written for this particular concept that you get as a starting point and that's in this file gold test general actually the test fails so at least it's a basic test and when I run Pi test you also see that the test actually fails and well the first step is to actually fix this test and that's pretty easy to do because it just asserts that the name of the item is fix me which of course it's wrong it should be full so this is a simple test to check that the name of the item is actually correct so let's also rename this test to something like test item doesn't change name like so now let's run the test again and now we see that we have one past test so that's great that's a good starting point so the first thing that I'm going to do is look again at these requirements and then write all the test so that we can be sure that the current code actually fulfills the requirements one thing we can check for example is that if we run update quality does the sell in value actually decreases so let's write a test for that and you can see that GitHub copied actually already suggests a test for that so this is going to test that the cell in decreases over time we can do another test where we also test that the item quality decreases so this also reminds that GitHub copied actually already suggests this for me there's also a couple of other generic things that we can test for example we have to make sure that the quality of an item is never negative and that the quality of an item is also never more than 50 so we can also write tests for that so here we start with an item with a cell in Day Zero and quality zero and then we're going to update equality and we're just going to check that the quality is still zero and same thing we can do with test that the item quality um is never more than 50. there we go now let's run those tests again and see what we have so now we have five past tests and we see that the code that we have actually passes these requirements and of course the test that I'm adding in these videos are still pretty basic you might want to add more edge cases you might even want to use a tool like hypothesis for property-based testing so that you get a way more uh robust set of tests to make sure the code is actually doing what you wanted to but for the video I'm keeping it relatively short you get the idea next what we need to do we have this generic tests now is that we need to write the test for these specific items because they have slightly different Behavior so let's start with H3 and as you can see I'm creating a separate test file for this because that way I find it easier to keep an overview of what does belong to what area of the code the specific thing that we need to check with HP is that it increases in quality the older it gets here's an example of what I test for that can look like so I've created an H3 item I'm using a constant value here just so I have to write the string only once even though there's only a single test we can add more tests here later update quality and I check that the quality is 2. and then we can run Pi test again to verify that this is actually also correct so here's the test passed as well and if you go back to the requirements we can also now write test for the sulfurous item and for backstage passes let's start with solvers that actually never has to be sold and also doesn't decrease in quality also here I'm going to add a file for that test solve for us there we go and to start working on this test of course we need to define the name of the item so that's what I'm doing here and now we can write our test for example we can test that the item so for us sell in doesn't decrease so I create the item I'm using the constant here and then I update the quality and similarly we can write a test that the quality doesn't decrease there we go and now let's run these tests so now we've also written the test for the sulfurous item the most complicated one is the backstage passes because that has like different Behavior depending on how many days there are left and what happens after the concert etc etc so let's create a new file test backstage.pi where we can write those tests now I already wrote these tests to keep this video relatively short otherwise it would take way too much time but basically these are the tests that I wrote for this so we have again the constant which is the name of the item so then we have to check that backstage passes increase in quality just like H3 we have a test that we checked that the quality increased by two when the sound is 10 or less that increases by 3 when it's 5 or less and that the quality is zero when sell in is zero as well and that basically follows these exact requirements that are written here so let's run the test one more time and indeed all of these 12 test tasks now again you might want to add more tests to check for other types of values maybe you don't want to just test items with these values you can use hypothesis for this you can write extra tests it's all up to you how far you want to go in robustness of the testing system but basically your building out your safety net your collection of tests that are going to make sure that whenever you change anything in the original code that you have your safety net to check that what you did doesn't break anything so we've now spent quite some time analyzing the code thinking about the concepts defining our goals writing a bunch of units as and we didn't even change a single line in the original code yet preparation really is half of the work but now that we have to test we can start to actually refactor this code and we're going to start by doing a few simple things to improve our quality of life basically so that refactoring later on becomes a bit easier one thing that we can do is split up things a bit so update quality rescue goes through a list of items using a for Loop and then it has all of this code what we can do is a simple refactor to split up the for Loop Behavior and the actual updating Behavior so what I'm going to do is create a new function here called update quality a single which is going to get an item and there we go and that contains this hole if else structure that we have here and then here we're simply going to call update quality signal very simple change but it does decrease the indentation here which means it's going to be easier to work on this code so we made the change now that we have the test we can simply run the test again make sure that we didn't break anything the next thing that I want to do is reorganize the code just a bit what happens in this function is that there's two things that are being updated the quality and the cell in days and these things are mixed up in the corporate we can kind of split them out a bit better so here you can see that there's a bunch of checks on the quality so this updates the quality this also updates the quality so if you look at all of disco this actually updates the quality this piece of code updates the cell in days and then this piece of code again updates the quality so what I'd like to do is move this to a different place so that we actually can split updating the selling days from updating the quality it's going to make it a bit easier to manage the only problem is that this change is going to affect the logic here and here so if we want to split it out we need to fix that first so if I move this to the top for example we also need to update these numbers here so here it decreases 7x1 that means we also need to decrease it here if we move this to the top so let me copy this and let me move this right to the top right here so you also see when I now run the test actually the test fails right because I move something and now we see that hey this actually goes wrong and that's of course because now we have to fix the numbers that are here so this should be 11 that should be 10 because we decreased it here and here it should be 5 instead of 6. so let's run the test again and now we see we've been able to move the code enough to fix enormous the test pass again so the good thing now is that we have updating the cell in here and the rest of the code is all about updating the quality and that's really good because now we can work on these things separately without having that dependency another thing that we should do in order to make refactoring easier later on is move these hard-coded string values out of the specific update function because you see they're being repeated here several times you might end up with typo so we also want to avoid that so let's go to the top of this file and then simply Define a number of constants Here and Now what I can do is replace all of these strings that I have here with the consoles so there we have it so now we have only a single place where we have to Define these strings which is nice as well and let me run the test again to make sure nothing breaks and this still works as expected one thing that we can do to simplify these if statements a bit is by looking at the requirements for Quality you see that there's a bunch of times that we checked that we only update the quality if it's larger than zero uh same thing here we also check that it's less than 50 and that's also something that you see here so instead of using IF statements everywhere to do these checks all the time we can also rewrite this and use a function that does that for us so I'm going to Define two helper functions and the first I'm going to call increase item quality and that's going to get an item and we're also going to provide it with an amount which is an integer and default that's going to be one because that's the default quality decrease and this is going to return none and you see GitHub Copart already helps me a bit with this so we set the quality of the item to the maximum of zero and item quality minus the amount so that makes sure the quality is always positive number and then we can also have a increase item quality and there we do sort of the same so we just update the quality of the item but we use the minimum of uh in this case the item quality and the amount and 50. and we can make this a bit nicer by not letting 50 be a hard-coded value but we could have a Max quality which is an end and by default we're going to assign that the value of 50. like so so this gives us an easy way to work with quality and we don't have to worry about these constraints that it should be between 0 and 50 because that's built into these functions so now we can start simplifying this update Quality Function a bit to use those functions so in this case we don't need this if statements anymore because that check is no longer needed that's handled by the function so I can actually delete that like so but then of course here I'm going decrease item quality like so and same thing here if item quality is less than 50 so we don't need that check anymore so I can just write here increase item quality like so and then I remove this whole if statement and we can decrease all of this like so and same thing here actually uh yeah so that's being replaced by increased item quality and same thing here we we don't have to do all of these checks because we can simply write here increase item quality and same thing here increase item quality like so let's run our tests to make sure we didn't break anything there still works correctly so let's continue here we have another check with an if statement we can remove that the indent this and call decrease item quality like so and then we don't need that and same thing here we don't need the if statements so I'm simply going to do increase item quality like so run the test again there we go still works not a minor thing we see here is that we can actually simplify this code a bit because quality minus quality is zero so let's just make that more explicit like so run the test again to make sure it still works and it does so now just by restructuring things slightly we've already simplified this update Quality Function quite a bit the next step is to untangle the logic a bit it's quite complex because there's lots of comparisons there's not equals comparison here there's an equals comparison here these things are nested so before we can split out things more we need to restructure this so that it's going to be simpler to work with and one technique that works quite well in order to do this is by in some cases inverting the logic and what I mean by that is that we now have a combination of these not equals comparisons and we have equals comparison so if we change that so that most of them are actually equals comparison then we just going to have a few cases we have the case of H3 we have the case of backstage passes and we have the case of sulfuras and we can see the code that we have in all of those various cases so how do we start with our well let's start at the top right where we have if the item isn't so far then this is what happens so actually this means that if the item is of type so for us then we're simply going to do nothing else we're going to do this so that means we have our default behavior and if it's soft for us then we're just going to do that so that covers the cell in days bits and just to be sure let's run the test again still works for the quality we can start doing the same thing let's start at the bottom here so here if the item isn't backstage passes then we're going to do this else we're going to do that so we can flip that so if the item name is backstage passes then we're going to do this like so else we're going to do this so we just reversed the logic let's run the test again still the same result now we can do the same thing on this level because here there's also a not equals so here if the item equals H3 then we're going to do this else else we're going to do this we're on the test again and then finally if you look at this whole if statement there's one more logic part that we can invert so here if the item is software as then we're going to do nothing else we're going to decrease the item quality let's run the test again there we go still passes now the nice thing of this is now we have a series of if else statements and we can now simplify that by changing that to an if alif chain so now I can basically do this decrease the endation indentation here and the same thing here has decreased the indentation here let's run the test again to make sure that this still works so what's nice about this is that we now have identified basically per item type what needs to happen we can do a similar thing here at the top so here we have a problem mainly in the beginning so let me first re-invert the logic here there we go run the test again to make sure we didn't break anything next thing that we can do is invert the logic here as well so if it's not H3 and it's not backstage passes then this is what happens else this is what happens so let's invert the logic so if it's H3 or if it's backstage passes so the ant becomes an or have to watch out for that then we're going to do this like so else we're going to do this and this else we can remove like so let's run the test again test still passes and now we can again change this into a single if a lift chain like so Let's test again still passes the only thing we have now is that there's still a piece of code that works on both H3 and backstage passes so I want to split this out add some duplication for the time being to uh to handle each case separately so what I'm going to do is I'm simply going to copy this code and I'm going to put that here like so and then I'm going to handle the first case if it's H3 then this is what we're going to do and if it's backstage passes then this is what I'm going to do run the test again to make sure that didn't break anything it didn't and now we can simplify things because if it's H3 then we know it's not backstage passes so this part is actually not needed if it's backstage passes I don't need this check here anymore so I can simply remove that decrease the indentation here let's run the test again still passes good so now what we have is we have an if else statement here that does the sell in bit and we have another if else statement here for the quality and we have another piece that also handles the quality so what I want to do is actually move this code here so that we have only one time a check for these different item types so here you see if item selling is less than zero and it's B then this is what we're going to do so I'm going to copy this and I'm going to put that right here and of course this check we don't need because we already know that it's H3 so now we have that right here and I'm going to do the same thing for the backstage passes so let me remove that because we don't need that anymore copy this for backstage passes so that's going to be here this check we no longer need because we are no it's a backstage pass so if someone is zero then quality is zero and we don't need that so for us we don't need to do anything so we can actually just delete that and then what we're going to do here that's the else part that belongs here so now we've changed the logic so that basically each item is handled separately we have H3 we have backstage passes we have software and we have the default behavior let's run the test again to make sure that we didn't break anything so now I feel already way more confident in dealing with this code because we have really split out the different behavior for each different item type but we haven't reached our goal yet because it's still kind of cumbersome to add new types of items what we need to do is introduce some abstraction so what I'm going to do is create a simple class hierarchy for updating quality and the sell-in days and then we have some default behavior and we can create specific subclasses for each of the different item types so I'm going to create a class called item update which will be a protocol and I want this class to have two methods we have update quality and we have update selling there we go you can do it GitHub copilot thank you and now we can do is create a default implementation of this default item updater and that's going to have the update quality and update so in methods actually gits of copies already does this for me so let's check that it's correct because I'm not entirely sure so uh the default Behavior should be this for selling so that's actually correct and default quality update should be this which is actually not correct so they see AI tools are nice but you always have to check what they do so that's the default update mechanism we can now also create a let's say aged 3 updater and we're going to inherit from the default item updater and the only thing that we need to do in H3 is change the behavior of update quality so this is going to increase the item quality and if the selling is less than zero then it increases twice as fast and that's also something that you can see right here so we have covered that case as well then we can add another class for the backstage passes updater and there we're going to also have a change in the update quality let's just check that this is correct so we increase the quality and then if it's less than 10 we do it again if it's less than 5 we do it again selling less than zero then the quality is going to be zero and that looks to be exactly the same good and then we're going to finally have our class so for us updater and this thing we just need to make sure that quality and cylinder is never changed so we just override both of these methods to just pass to nothing so then what I'm going to do is I'm going to create a dictionary called item updators and that's going to be a mapping from each of these consoles that we have H3 backstage passes to the different updates or types there we go and then what we can do is step by step replace this complicated code by simply calling the item updators so first step is getting the item updater I'm using the dictionary here to get the updater based on the item name and if it can't find out it's going to supply the default item updater there we go now I just want to be sure that in the current state I added some code to it I didn't really change the function just want to make sure that the tests still pass and they do so that's great and now what I can do is start moving each of the different cases to use this new mechanism of updating the item and for example I can start by replacing the selling part so I'm going to do item updater update selling and we're going to pass it the item and then this actually we no longer need let's check that this still works and this seems to work correctly so that's great and now what we can do is take this entire if else statement here delete it and also updates the quality like so run the test again so this is a pretty big change still passes so I guess I got lucky otherwise of course I would have been able to go back to previous version and check where the problem was and you can also see that it actually uses these tests because for example let's say that I forgot to override this method in the software's update so when we run the test again then we see that actually the test fails now this type of item hierarchies is one of the many ways in which you can improve the structure of your python code my online course on software design contains many more techniques and tips to help you write better code if you want to learn more about that check the link below so now I think we're finally ready to add a new item type and I think we've reads a goal because that's now pretty easy because we now have a simple class structure so let's take a look another look at the requirements so according to the requirements we need a new concert item that degrades in quality twice as fast as normal items so in order to add the new item well we're first going to need to add the test so let's create a test concert and I already created this test to save you some time waiting for me to write it and here you can see that that's the test that checks that Contour degrades twice as fast now when I run the test obviously the test is going to fail because it didn't Implement anything but that's actually nice because now we can follow a red green refactor kind of approach test driven development kind of approach where you first write the test and then we actually write the code until we are sure that the test passes the first thing we need to do is Define the string constants for this particular item it's the conjured item and then we can create an update our class for this particular item so we're going here and I'm going here to create a new class Contours updater and as you can see GitHub Copart already helped me out a bit in that we decrease the item quality with two so that's twice as much and of course and if the selling days is less than zero then it's going to decrease it again so let's run the test again and see if that's solved it well it didn't of course because we not only need to create the class we also need to supply it to the item updators so we're just going to add the concert updater here and let's run the test one more time and now we see that the test pass now there's a couple of other things you can do as well to further increase the quality of the code we've reached our goal it's now relatively easy to add a new item type to the code but you could improve this more by for example splitting out these different classes into different modules so that not everything is in a single large file you could even imagine that there's some things that you can read from a config file like the names of the item types and things like that I didn't do it for this particular refactoring because I think the video is already getting quite long but there's definitely more things that you can do to make things even better so that completes the refactoring work the framework I used for this had five steps the first step is analyze you need to analyze the code understand the concepts and the relationships so that you have clear understanding of how everything is supposed to work you saw that I looked at the requirements I looked at the code try to understand what the concepts were the second step is goal setting before you start refactoring anything you need to understand what your goal is what are you trying to achieve why are you modifying the code at all what do you want to do and related to that you need a measure of success you need to know when you're done refactoring in this particular example the measure of success is am I able to add a new item type easily the third step is to create the safety net how do you do that by writing a bunch of units as to check that the current version of the code is working as expected and you want to write the test so that it covers the most Behavior possible don't skimp on this don't think oh you know what I just write the test for H3 and then I'm going to change things and then when I'm going to work on the other parts of the application I'm going to write the test for backstage passes or whatever that doesn't work because there's all kinds of Connections in the code that you don't fully understand and by changing something in one place it will have effect on other things as well so you need to complete set of tests before you start changing anything in the code it's like creating a safety net and then attaching only half of it you know you're still going to die if you do that so write a complete set of tests before you change anything and verify that these tests actually pass so that you have a working starting point then step four is finally refactoring and you do that step by step and you can use a couple of basic techniques that I've used in this refactoring as well the first is to start with simple quality of life Improvement changes first like for example moving out the hard-coded strings into constants so that I wouldn't have to worry about typos anymore later on sometimes you might even make a change in the code that you're going to throw away later it's just an intermediate step in refactoring you also saw that in how I worked for example I first moved everything to a if alif chain statement but then I threw that out of the window again as soon as I moved to the inheritance hierarchy but I could only write the code with the inheritance hierarchy after I've split out things more and restructured things so sometimes you may write code that you will throw again throw away again later because it's an intermediate step that you're going to need but go from simple to more complex a second thing you've seen me doing refactoring work is inverting the logic so sometimes if you have code with complicated logic lots of interconnections then it's helpful to actually restructure things and invert the logic turn conditions in an if statement around so that the order changes and it allows you to get a better overview of how everything works together a third thing that you saw me do is to split isolated Parts out into separate phone that makes sure that when you're working on a piece of code that you don't have to worry about that other parts that was that doesn't have anything to do with the code that you work on it's in a separate function and then you can also write separate tests for that if you want so it's going to make it easier to manage and finally something that I didn't show in the video but I still highly recommend is that you commit your intermediate steps so that if you make an error somewhere and you want to go back to a previous state you can always revert a commit if you're running it repo story and then go back to a place where everything was still working and then of course while you're refactoring make sure that you always run your unit test because that's your safety net that's your check that everything is still working and the final step the final step step number five is to measure your success and verify that the goal that you set out for yourself is actually reached in this case my goal was that it would be easy to add a new type of item so I actually added a new type of item to check whether that was actually true so those are the five steps and analyze set your goals create the safety net refactor step by step and measure whether you were successful at the end it was really fun working on this code refactoring so how do you think I did anything you would have done differently any suggestions let me know in the comments and if you're working on a project that involves refactoring Legacy code try the framework that I just taught you if you enjoyed this video you learned something from it you might also enjoy my code roast Series where I go through code submitted by you and refactor it and try to solve most of the problems in it thanks for watching and see you soon

Channel: ArjanCodes

Views: 33,282

Keywords: refactoring, code roast, code roast arjan, code roast python, refactoring code, python code roast, python code review, refactoring code python, refactoring python, refactoring python code, gilded rose kata, gilded rose, gilded rose kata python, gilded rose kata solution, gilded rose refactoring, refactoring legacy code, python code review tools, refactoring methods, refactoring tutorial, refactoring framework, refactoring code explained, refactoring code best practices

Id: Rryo6CoKamE

Length: 35min 57sec (2157 seconds)

Published: Fri Sep 01 2023