List

Down The Rabbit Hole: An Adventure in Legacy Code

Down The Rabbit Hole: An Adventure in Legacy Code

by Loren Crawford

In the talk "Down The Rabbit Hole: An Adventure in Legacy Code" at RailsConf 2018, Loren Crawford explores the complexities and challenges of working with legacy code while offering practical strategies for improvement. The main theme revolves around understanding legacy code, its pain points, and effective methods for enhancing it.

Key points discussed include:
- Definition of Legacy Code: Crawford defines legacy code as working software that is difficult to change and often contains complex business logic written by various developers over time. Legacy code, while cumbersome, often signifies the success of a company.

- When Not to Change Legacy Code: Highlighting Sandi Metz's advice, Crawford suggests that certain pieces of code, if seldom accessed, may not merit changes due to the risks of introducing bugs.
- Reasons to Change Legacy Code: Using Michael Feathers' framework, the speaker outlines four primary reasons for altering legacy code: adding features, fixing bugs, optimizing performance, and refactoring.
- Add Tests Before Changes: Crawford advocates for adding tests before modifying existing code to ensure current functionality remains intact. He emphasizes that tests serve as documentation and help prevent regressions.
- Incremental Improvements: The talk underscores the importance of incremental improvements to legacy code by integrating enhancements while addressing new functionalities, rather than setting aside time for large refactoring efforts.
- Testing and Bug Fixing: Crawford shares methods for writing tests to replicate bugs and ensure that changes made to the codebase do not disrupt existing functionalities.

Throughout the talk, Crawford shares practical examples, such as an online yarn marketplace where new features and bug fixes are addressed through proper testing strategies. He illustrates how to add tests for searching specific yarn products and handling unexpected inputs.

In conclusion, the key takeaways from Crawford’s presentation include:
- The value of understanding and embracing legacy code as a part of successful software development.
- The necessity of writing tests for untested code to enhance knowledge and documentation of the codebase.
- The importance of making incremental improvements to legacy code, rather than attempting to refactor everything at once.
- Encouragement to seek help and foster collaborative efforts within development teams for better handling of legacy code.

Crawford recommends several resources for further reading, including works by Michael Feathers and Martin Fowler, to support ongoing education about legacy code management.

RailsConf 2018: Down The Rabbit Hole: An Adventure in Legacy Code by Loren Crawford

Legacy code is unpredictable and difficult to understand. Most of us will work with legacy code, so it’s important for us to understand its major pain points and their underlying causes. You’ ll come away from this talk with practical strategies to help you understand, improve, and even love legacy code.

RailsConf 2018

00:00:11.240 okay thank you okay I want to take a little bit of time just to talk to you a little bit about me so I'm a software
00:00:18.660 engineer at MailChimp in Atlanta Georgia throughout my career I have developed a
00:00:24.510 love refactoring and testing specifically tested on development and I
00:00:29.970 have a wonderful dog Luna I'm just I can't help but share pictures of if you
00:00:35.489 want to reach me out in the Internet's you can reach me at twitter at el crawfish so what are we going to talk
00:00:44.129 about today you probably saw that I was going to talk about legacy code and I'm
00:00:49.980 not tricking you we are gonna talk a little bit about what's up with legacy code go into what I define as legacy
00:00:57.960 code and the premise of this whole talk then I'm going to go over some pain points illustrated by adding a feature
00:01:04.350 to some legacy code I built and fixing a
00:01:09.420 bug after that I am gonna review some lessons I hope that you learned today and give you some other recommendations
00:01:19.850 so let's begin what's up with legacy code can I get um can I get some hands
00:01:27.360 in the air if you currently work in legacy code oh yeah have you have you
00:01:34.770 ever felt like Ron Swanson anyone I
00:01:40.100 certainly have and so it seems like a lot of us work on legacy code and we've
00:01:46.560 all kind of encountered these these really frustrating moments where we don't know why it's happening we don't
00:01:52.110 know what's what's even broke but to begin with I wanted to also talk a
00:01:59.909 little bit about why legacy code isn't that bad in the beginning of my career I
00:02:07.920 would look at some code I was worth none and I would immediately be like who did
00:02:13.520 gee Blaine right now and then I'm like whispering like who's Tim like who is
00:02:19.650 this guy and you know I had like this bad connotation of people that made this horrible code and but as I've grown I
00:02:27.860 I've come to realize that the people that came before me operated under a lot of different constraints than I did they
00:02:35.370 had different information they were certainly under different time constraints than I am now and generally
00:02:42.390 they were trying to do the best with what they had so I want to just get it out there that legacy code isn't awful
00:02:50.790 and in fact if you're working in legacy code it means that you're probably at a pretty successful company at least they
00:02:57.750 make enough money to pay you hopefully so it's somewhat successful and I'd like
00:03:05.430 to quote one of my colleagues for MailChimp Dustin shields Klaus he says on Twitter when someone makes fun of
00:03:12.299 legacy code remember legacy is a funny way to spell successful your startup
00:03:17.820 with perfect code will fail and this again demonstrates a good point about
00:03:23.370 legacy code it's cumbersome curmudgeon II and doesn't always work the way we
00:03:30.780 think it should but it's still successful and it was still written by people that contributed to that success
00:03:41.780 so now that I've kind of laying it out laid it out that legacy carriers and all
00:03:47.220 that bad I want to tell you what my definition of legacy code is legacy code
00:03:53.880 is a system of working software written by many developers over time that is
00:04:00.780 often hard to change and contains business logic complexities that are not
00:04:06.030 easily distilled from the code so at the bottom of this slide you'll notice I
00:04:11.250 have a working definition and the reason I put that there is because in the
00:04:16.560 beginning of my career I would have had a very different definition of legacy code it might have had some not-so-nice
00:04:23.099 words in it but as I've grown my career I've widened my definition and
00:04:29.860 I'm sure in the next five to ten years I will have a different definition so this
00:04:38.919 talk is about changing legacy code how do you handle it how do you wrangle it but I also want to mention something
00:04:45.370 about when not to change so we're talking about lots of change of legacy code but when to think about not
00:04:52.120 changing it and I'm going to kind of stand on the sand on the shoulders of
00:04:57.190 giants today and paraphrase Sandi Metz
00:05:02.310 she told me one time that if you have this like really ugly piece of code and
00:05:07.960 like nested ifs and there's like a switch statement if no one ever opens
00:05:14.110 that file if no one ever reads that don't change it the cost of changing it
00:05:20.830 that caught the potential for introducing bugs and risks is very risky and if no one's reading it then you're
00:05:28.240 not reaping any of the rewards of changing software unless of course it's
00:05:34.120 broken so Michael feathers in his book working
00:05:39.310 effectively with legacy code breaks up four reasons why you would change legacy
00:05:44.740 code we've got adding a feature which is probably what a lot of us do all the time fixing bugs hopefully we don't do too
00:05:52.270 much of that optimizing performance and also refactoring I've taken these four
00:05:59.380 and I've broken them up into changing behavior and improving design and the
00:06:04.449 reason why I break them up and lead to these two components or these two buckets is because I believe that you
00:06:11.860 can actually improve design as you change behavior and it's a more pragmatic way of changing and improving
00:06:18.639 the design of your application I'm sure
00:06:24.039 many of us work in businesses where we need to add features we need to fix bugs and asking the business to give you a
00:06:31.389 couple of months to refactor this really gnarly part of the code is probably not
00:06:36.580 going to happen and so I would encourage you to take the opportunity as as you change behavior in
00:06:42.810 little bits of your code that you're improving design and when I say improving design I mean adding tests and
00:06:49.379 I also mean refactoring which we'll get into a little bit later so let's go down
00:06:54.689 the rabbit hole first we're going to add
00:07:00.629 a feature and today I'm gonna operate under this example that we work at this
00:07:07.259 place called yarn hearts it's an online yarn marketplace where people from all over the world can buy yarn they can buy
00:07:14.610 different types of yarn for all different types of things and you don't need to know a lot about yarn to
00:07:20.699 understand this of course we've got plenty of examples out there that you can draw from so this part of the talk
00:07:30.270 we're talking about adding a feature so let's talk about requirements of our feature we have users who want to be
00:07:37.020 able to search our application for online sellers that sell alpaca yarn
00:07:43.430 this alpaca yarn also needs to be fair trade and it needs to be organic whatever is returned so we have these
00:07:50.099 requirements in my contrived example so what do you do I'm going to tell you
00:07:59.219 first to look at the tests tests are a great way to great place to start and
00:08:06.270 especially in rails you've already got this built-in test directory along with these sub directories controllers
00:08:12.509 mailers systems models all of that you could also be using r-spec in which case
00:08:18.389 you're under the directory spec but either way hopefully you have somewhat of a good directory full of tests and if
00:08:25.919 you don't really know where you should be looking think about what kind of object would I be calling what is it
00:08:31.349 going to do is it going to change data is it going to change something view that sort of thing
00:08:37.829 you may also want to reach out to your teammates if you have no idea what
00:08:43.689 you're doing in this new codebase say you're like the second week teammates
00:08:49.720 can be a great resource with legacy code as I'm sure a lot of you know there are
00:08:54.910 some people that have probably been around your company for a while and they're the ones that kind of contain a
00:09:00.189 lot of the information the knowledge unfortunately it's just stored in their brain for right now maybe we'll come up
00:09:06.040 with a way we can extract that out easier than just getting them to write stuff down but the knowledge that it's
00:09:14.679 better in people's heads are is incredibly valuable so make sure you're doing that but for the sake of this
00:09:21.339 example so we're adding a feature let's assume that our Lucy carrot has some they're tests around it I'm gonna quote
00:09:30.970 Michael feathers again it says he says rather it's eat it's better to confront
00:09:37.300 the Beast than the hide from it and I'm taking this completely out of context so you didn't have to believe me when I say
00:09:42.790 that what this is referring to is when you have no tests surrounding current
00:09:48.850 functionality you should stop and add tests to preserve current functionality
00:09:58.180 so let's go ahead and add test Reserve current behavior you may be thinking why
00:10:05.690 should I be testing hopefully all of you understand the value of testing but you
00:10:11.480 may not be fully convinced so why I test is I want something to be able to prove
00:10:18.649 that my code works and that the assumptions I'm making about my code work or are correct rather testing also
00:10:27.709 helps you to uncover two real bugs for example I don't know how many times that
00:10:32.990 tests have helped me to uncover a bug where I expect something to be accepting
00:10:38.449 a string as a parameter and sometimes it's getting an integer or something like that
00:10:43.750 another reason why I test is it's a great way to document how your code is
00:10:51.079 actually working I've been in places where there is sort of a divergence
00:10:56.930 between how codes should be working and how it actually works and that can be kind of confusing for someone who's new
00:11:03.920 to the code base or even you'd like the area of code that they're working in and
00:11:09.190 finally testing helps you design better code if something's hard to test chances
00:11:17.959 are it's not as well designed as it could be
00:11:23.420 we've all been in that situation we just don't have time I do not have time to go
00:11:29.070 into that test file and add the tests no no no no no I need this I need to do
00:11:34.170 this other this other ticket first I'm going to say that I hope I think that
00:11:40.589 you might have the time just gonna go out on that limb and I'm also gonna say
00:11:46.050 this so since we are going down the rabbit hole I'm going to quote Lewis Carroll he wrote that the white rabbit
00:11:53.339 always the right rabbit said in Alice in Wonderland the hurry I go behind her I
00:11:59.670 get and this is sort of me cautioning you against saying like if you don't
00:12:04.890 have the time that's fine but that's a trade-off you're making a decision that
00:12:11.600 the tech debt that you are putting down
00:12:16.740 is going to need to be repaid you're kind of making a contract hopefully your
00:12:22.440 legacy code is successful and someone gets to touch that part of the code again and I think you and your team need
00:12:30.029 to fully realize that this will decay and you're not improving
00:12:37.740 necessarily the design of your code you're not improving it for other people so it's a trade-off also I have to
00:12:45.600 suggest if you don't have a lot of time but you'd like to kind of test whatever
00:12:50.940 you're adding working effectively with legacy code again Michael feathers he has a chapter all on this that you don't
00:12:57.510 have the time but you have to change an area of the code so I recommend reading that as well okay so example time I've
00:13:07.350 done a lot of talking let's look at some code so again our requirements our users
00:13:13.140 want to be able to search online sellers for alpaca yarn and all the yarn that is
00:13:18.210 returned or the sellers that are returned are our fair trade and organic
00:13:23.750 so we look into our magical legacy code
00:13:28.950 that I've created and we have this yarn model and we look in the database or we
00:13:34.680 look in the model and we see that it has yarn name gauge weight color is organic
00:13:42.990 yours is Fairtrade and seller ID now I'm thinking seller ID is probably a foreign
00:13:49.080 ID so I look at seller look at the seller table and it has name address in
00:13:55.860 good standing is organic is fair trade and I find that the yarn model has this
00:14:03.680 class method called get sellers so I'm
00:14:10.590 adding this feature I think I know about where it needs to be it needs to be and
00:14:17.000 and I look at the code please don't read this don't try to do this it's too much
00:14:22.070 but I like if you squint you can see like a lot of nested code you've got a
00:14:27.180 switch statement that's those are a lot of queries you've got some nested it's
00:14:33.720 looks pretty gnarly so again you've got this you've got some cotton silk mohair
00:14:39.990 to cashmere synthetic gorya these look like yarn types if you don't know that they
00:14:45.209 they googled it they are but so you've got all this the switch stuff happening
00:14:52.350 oh dear and and I'm telling you you need to add tests to reserve all this current
00:14:58.019 behavior it's a bit of a doozy right it's a lot to take in but we're gonna do it together the first thing
00:15:06.509 that I'm going to suggest that we do is look at the like the most innermost logic around what we saw in the first
00:15:13.769 bit of the switch statement which is a round wool so we're in the yarn model we
00:15:23.550 are in a method called get sellers it takes a name and some optional params we
00:15:30.839 are defining an empty array found sellers before we go into the case of the switch statement within this we have
00:15:40.439 some found sellers so presumably we we took that empty array and defined it
00:15:45.540 into something that is not an empty array and we iterate over all of those
00:15:50.879 sellers we check if they are in good standing we check if they are organic in
00:15:56.699 our fair trade and if that's true then
00:16:01.980 we are going to place that seller in a preferred seller its array if they are
00:16:07.259 organic but they are not fair trade we are going to put them in a new array called mid sellers oh okay that's a lot
00:16:15.350 and you may be thinking like Lauren we're adding a feature don't tell me about what code is already doing but
00:16:21.209 this is an important part to kind of see like what the code is doing and what people were thinking of while they were
00:16:27.779 writing it we also in this code do a
00:16:33.509 little bit more after that loop around found sellers we if preferred sellers if
00:16:42.089 the count for preferred sellers is greater than the mid sellers count then we want to return folks preferred and
00:16:49.049 mid together making sure that the mid sellers is is after the preferred sellers
00:16:55.490 and if that's not the case we just want to return all of the found sellers so
00:17:02.889 we're adding tests to preserve current behavior before we add the feature first
00:17:09.110 we're going to write the simplest test I can think of first we're going to look back so we know that in order to kind of
00:17:17.899 get set up with this test we're going to need to set up the right kind of seller and to point this out so we need a
00:17:24.260 seller that is in good standing a seller that's organic and a seller that's Fairtrade in order to get some preferred
00:17:31.250 sellers so let's start out I've got a context when you pass in wool
00:17:40.759 I'm sorry and we're describing get seller the the method and we're passing
00:17:47.100 wool into that so when you pass in wool as yard name to get seller it returns
00:17:53.279 one found seller so that is my test so I need to create that one seller again the
00:17:59.549 the seller needs to be organic fair trade and in good standing and then I
00:18:06.360 need to create a instance of yarn with that is that has the name wool and that
00:18:13.950 has the seller that I just created attached to it the yarn also needs to be
00:18:23.990 organic and fair trade so I have the setup I've got my seller I've got my
00:18:29.909 yarn it should pass so I go ahead and run the method under test so found
00:18:36.779 sellers equals yarn dot get sellers passing in wool and then I have my
00:18:42.899 expectations I expect that we get one seller back and that it should equal the
00:18:48.389 seller that I created and set up and it passes so we're adding tests to preserve
00:18:54.659 the current behavior I want to go back here so the reason why I would suggest
00:19:01.279 adding the tests first because originally and let me back up in my earlier career I would think oh we need
00:19:08.009 to we need to fix this but I I really encourage you if you do want to if you
00:19:13.440 do see areas like this and you want to improve it to make sure you're adding the tests first so that you don't
00:19:19.320 inadvertently break something so let's add another test so we need to also
00:19:24.360 think about mid sellers so in our my previous example I just created preferred sellers but this time we need
00:19:31.169 both preferred sellers and mid sellers and mid sellers needs to be at the end
00:19:36.450 so preferred sellers needs to be at the beginning
00:19:45.440 so I read another test the context when you have more preferred sellers than mid
00:19:51.360 sellers it returns an array with preferred sellers before mid sellers so
00:19:57.749 again I've got to do my setup I need a preferred seller that is organic and is fair trade a second one
00:20:04.259 of the same type then I need a mid seller and a mid seller is organic but is not fair trade then I need to create
00:20:14.549 yarn instances that match those sellers or that I'm sorry are attached to those sellers so I've got those there next
00:20:24.179 we're going to call I'm going to call get sellers on yarn passing in wool and then finally my expectations so I expect
00:20:32.039 that this method will return three objects in its array and that the found
00:20:38.669 sellers the last bit of found sellers is the mid seller and it passes
00:20:54.720 now you may be thinking like I've talked a lot about refactoring and testing and
00:21:01.290 and you you may not still be convinced because this car is gonna get difficult
00:21:06.930 to test kind of quickly but the reason why I suggest you go through all of this
00:21:13.860 is because a legacy code that's why we're here today
00:21:18.900 improving legacy code if you're able to add tests to a certain certain area of
00:21:25.500 the code base even if you can't refactor it you've left it better than you found it and if we incrementally improve
00:21:34.020 legacy code then eventually it won't be this gnarly kind of weird come under new thing that doesn't do the things that we
00:21:40.020 want it to do I'm going to quote Martin
00:21:46.740 Fowler and his from his book refactoring and although we're not doing refactoring
00:21:52.410 right now in this example he says whenever I do refactoring the first step is always the same I know I need a I
00:21:58.740 need to build a solid set of tests for that section of code the tests are
00:22:03.930 essential because even though I am following refactorings structured to avoid most of the opportunities for
00:22:09.900 interest introducing bugs I'm still human and still make mistakes
00:22:16.970 so we're all human hopefully well if not maybe it's an upgrade I don't know but
00:22:23.370 we're human and we make mistakes and that's why we should be adding tests if
00:22:29.640 we have any hope of refactoring so I've
00:22:35.460 added tests to preserve current behavior we saw have to add this feature thing they pay us for so again
00:22:41.490 users want us to be able to search online sellers for all packing yarn and that Pocky yarn has to be fair trade and
00:22:48.150 organic so let's add a test I did say
00:22:53.280 that I was and I loved test-driven development so we're gonna add a test first so this is my test context when
00:23:01.110 you pass in alpaca as the yarn name it returns sellers with only yarn that is fair trade and organic and to be more
00:23:08.700 specific this should say it returns sellers with only alpaca yarn that is
00:23:14.400 fair trade and organic so like the other tests we have to build some setup so you
00:23:21.390 have one seller that should be included that is organic and fair trade and a seller that's not included that is or
00:23:27.510 not organic and is not fair trade again
00:23:37.200 I go ahead and create the yarn attached to the sellers I call the method get
00:23:45.090 sellers passing in alpaca and I make this assertion that we should
00:23:50.970 find one seller and it should be equal to the right seller that we put in our setup and the test fails and this is
00:23:59.640 good that's why I have thumbs up here because this is you've captured your requirements and a test so you know that
00:24:06.150 when it passes that you've done your job specifically this is the error that you
00:24:12.180 get undefined method count for nil class and just to go back here that means that
00:24:17.970 found sellers is nil so that means that nothing's being returned when we make the call and that makes sense since we
00:24:24.840 haven't implemented it yet so I go into the big gnarly method that
00:24:31.669 I added tests for and I go ahead and add another switch statement with alpaca and
00:24:37.750 I get the sellers where the yarns name
00:24:43.610 is alpaca and I'm joining that to the yarn table and then I do some one thing
00:24:52.610 that I saw on the other examples with the switch Kate switch statement i define find seller found sellers and
00:25:00.200 iterate over each one there's a typo there it should say found sellers set each instead of sellers and then for
00:25:09.559 each seller I check if they're organic and fair trade and I return and I add
00:25:17.179 them to found sellers and then return found sellers in the test path which is
00:25:23.539 good but the cool thing too is that looking at this they're probably better
00:25:29.840 ways to do this you I could have probably passed in that is organic and
00:25:35.210 is fair trade into my query at the beginning and not even had to iterate
00:25:40.220 over all of these but I have a test and when I go back and improve this code I
00:25:45.409 know it's it's still gonna work because my test is gonna pass but that's only
00:25:51.470 the happy path so this is a contrived example of course but there may be other constraints that you're working with
00:25:57.730 edge cases where maybe something can be organic but then something new comes out
00:26:04.610 and they have to be that to their educators all over legacy code and
00:26:09.620 making sure that when you add tests that you're not just thinking about well this is the happy path let's go ahead and
00:26:15.230 like we did our tests we did our due diligence but also think about the not
00:26:20.840 so happy path what happens if something wasn't passed in what happens if an error is thrown should an error be
00:26:27.529 thrown
00:26:39.600 so I'm gonna flash this slide back up here and don't try and read it so we are
00:26:48.250 still in our add a feature phase of this talk and we did a lot we
00:26:53.320 added we saw that there was a lot going on in this method called to get sellers
00:26:59.320 a lot of nested logic but we added tests for all of that like let's assume that those two tests
00:27:05.020 were enough to just cover everything so we did our due diligence we tested the current behavior we added our our tests
00:27:12.250 for the feature and then we added code for our feature as well but then we go
00:27:17.620 back here and like we have this reaction I don't know about y'all but I've been
00:27:24.070 in some code bases where I see these really long methods and I'm just like I don't know I don't know how to do this I
00:27:29.380 don't I don't you know it's just a lot to take in but the beauty of it is that if you've kind of followed these steps
00:27:39.540 you will actually be set up to refactor quite well because you have this be sweet this suite of tests that are
00:27:45.970 automated that you can run and Wow you can go ahead and refactor it you may you
00:27:53.890 may want to pull something out into a service object you may want to maybe
00:28:00.520 create different subclasses there are other opportunities out there for you to change the code it will of course change
00:28:06.880 some of the tests but ultimately you'll be more confident in your changes so I
00:28:13.270 have an asterisk by this by this word refactor and I and it has if you can at
00:28:20.500 the bottom and the reason why I would totally be gung-ho about like write all the tests I have the feature refactor
00:28:28.420 but we don't all have that time because probably your ticket should have been
00:28:34.360 done yesterday and so we don't always have time to refactor but and actually
00:28:42.430 you you shouldn't always refactor I think that I've been in a couple of code
00:28:49.030 bases where there are some really big parts of the code that make us a lot of money and to
00:28:55.470 go into that and refactor it is a huge risk because even if it's a small bug it
00:29:03.240 can impact how much money your company is bringing in and no one really likes
00:29:08.790 shipping bad code either so I strongly recommend if you are going to try and
00:29:14.670 refactor something that's makes makes your company a lot of money or and it can it's particularly difficult to
00:29:22.230 understand refactoring would be best be done we're in a collaborative effort with people
00:29:29.400 who know that area of the code base a lot a lot more what refactoring is good
00:29:35.580 and if you have the time do it the reason why party cover now we go into
00:29:44.880 kind of like how we refactor some of you might already know and I've mentioned this before but in our example look for
00:29:51.720 look for opportunities to split up the code for example this switch statement
00:29:57.030 might be a really good example or a really good opportunity to replace the
00:30:04.200 conditional with polymorphism you may even want to do subclasses you may even
00:30:09.780 want plus the extracting out into subclasses you may even have like additional objects that you need to that
00:30:16.380 you could pull out and and separately test while present preserving the
00:30:24.060 current test and your current behavior of the method you're in there now or
00:30:29.760 you're working in now all right so we've talked about legacy
00:30:36.100 code we've talked about some of the pain points when adding a feature now let's go ahead and fix a bug I'm not going to
00:30:44.320 talk about how to debug that in and of itself kind of deserves its own talk so I'm going to recommend that you watch a
00:30:50.530 common taxonomy of bugs and how to catch them by my colleague Kyle East Radley it's a really great talk and covers a
00:30:58.270 lot of the stuff that I'm not going to cover here so in our example so imagine
00:31:04.179 we have this bug that there are sellers with silk blend yarns that are not being returned when a certain when the seller
00:31:10.720 searches for silk and that and that should be happening so first we're gonna
00:31:16.330 write a test to duplicate the bug very similar to before context so when you
00:31:23.320 pass in silk as the yarn name you it
00:31:28.720 returns sellers with silk and silk blend yarns so we're gonna start off with
00:31:34.000 setup so we're gonna create two sellers one seller is just silk just so silk and
00:31:39.760 then the second seller has it so Flint both of them are organic we're going to
00:31:46.809 again create yarn that's attached to these sellers and make sure that one is silk and one is soap blend
00:31:54.670 so we've got the set up now we're going to call our method again yarn gets
00:32:00.770 sellers and this time we're passing in silk and we expect to find two sellers and we expect that the silk blend seller
00:32:08.330 is included in the returned sellers the test fails so good we've effectively
00:32:16.040 replicated the bug that we were seeing this is the specific air and this is
00:32:22.159 exactly what we expect we expect to only be getting the seller that has the silk yarn and not the silk blend so this is
00:32:30.109 when we change the code to make our test pass so this is the original code where
00:32:38.509 we are getting sellers and joining on to the yarn table with the name just silk
00:32:43.909 and so what I add is not only silk for the name but also so Flynn and the tests
00:32:49.879 pass but you may remember that this part of the code is quite long and the circle
00:32:57.979 part there shows that I'm only addressing part of the problem because
00:33:03.789 we are grabbing the name of silk here here here
00:33:10.039 all the way down all that that's a lot so I'm only addressing one part of the
00:33:15.649 problem so I think about what other ways how other how this felt this how else
00:33:22.580 this could go so first I look that in the first if statement we are also
00:33:29.149 passing engage so I'm gonna add a test
00:33:35.149 very similar to the previous one so I've got a seller and a soap lens seller but this time I make sure that the yarn that
00:33:42.289 I'm creating has a gauge of six I pass not only silk in but six into the get
00:33:48.559 sellers method and I expect there to be I expect there to be two sellers and it
00:33:59.089 to be include the silk blend sellers to be included and it fails again this is the same
00:34:07.660 error we got before so I go ahead and add my solution and the test path now
00:34:13.900 let's imagine that we did that for all of the else if statements it's kind of
00:34:19.150 annoying right so this might be your opportunity to refactor and again like
00:34:24.250 my argument here is that we can improve legacy code and increments pragmatically when you're changing part of the code
00:34:30.790 you're also improving design and this would be a great opportunity for you to do that in which there are so many other
00:34:38.680 more optimal ways than wedging all of that into and if-elsif statement so
00:34:49.420 after after we've written the test to replicate the bug fix the code so that
00:34:56.800 the tests pass we want to run all of the tests we want to make sure that we
00:35:02.710 didn't break anything I've had the experience of kind of like being in spaghetti code where you start pulling
00:35:08.110 here and you kind of get everything ready here and then like over here there's even big a more of a bigger kind of not occurring so running all of the
00:35:15.640 tests especially if you have good test coverage can give you more confidence in pushing your changes so what if
00:35:24.190 something broke when you ran all the tests first this is good I think that it means that you might have a really good
00:35:29.710 test suite and you didn't you didn't go ahead and ship bad code
00:35:37.230 secondly go ahead and look at what failed and why did it fail did it fail
00:35:43.720 because it's a flaky test and it always fails did it fail because you
00:35:51.000 accidentally changed some logic that you expected to be fine so look at what failed and if you think
00:35:58.990 that it might be the code that you added go ahead and check your assumptions on your new code and see if it aligns with
00:36:07.330 what really needs to be implemented okay so we've talked about legacy code
00:36:14.520 the pain points adding a feature fixing a bug here are some lessons that I hope
00:36:19.799 that you learned firstly add tests when you touch code that is currently
00:36:24.809 untested your fellow developers will thank you bonus is that if you are
00:36:32.430 adding tests to parts of the code that are not tested currently you're gonna
00:36:37.589 learn that area of the code a lot faster than you would eat otherwise because learning how to test something means
00:36:43.049 that you understand the problem I'm also going to suggest that we refactor to
00:36:49.380 improve design incremental e and I've kind of been harping on this the whole time but we're rarely going to get a lot
00:36:56.640 of time to refactor big parts of our code so if we can do it in small increments and kind of wedge them in
00:37:02.010 there it's the best way to improve our legacy code ask for help sometimes our teammates are the best
00:37:08.940 sources for help add tests for new
00:37:14.369 functionality and that's all of my lessons learned and here my
00:37:20.460 recommendation so I talked a lot about Michael feathers working effectively in
00:37:25.710 legacy code I also mentioned refactoring by Martin Fowler both of those are great I also recommend object-oriented design
00:37:32.819 and Ruby and apprenticeship patterns in addition to those books I recommend the
00:37:38.130 talk by Kyle East Radley a common taxonomy of bugs and how to catch them additionally get a whiff by sandy met
00:37:45.599 which heavily relies on the code smells described in the refactoring book and
00:37:53.279 finally if you want to learn more about code smells right now you can go to source making comm slash refactoring
00:37:59.160 flash smells I'd like to take a quick moment just to
00:38:04.170 thank all the people that helped me to be here today including my conference
00:38:09.270 buddy Megan to my co-workers Tim Hayes Kylie Strad Lee Michael herd and Chelsea
00:38:17.849 yeoman and finally my wonderful and supportive husband Gabriel Holly thank you very much