As part of my summer learning plan, I also wanted to practice refactoring and test driven development. I have been doing the Gilded Rose refactoring kata for the past 3 days and I have to say that it really is a great way to practice. After every iteration, I noticed an improvement in the code that I was producing and also in the way that I arrive at the solution.
I put my code up on github but it was only in the recent days that i started creating branches for the solutions that I got. In hindsight, I should have done this from the start as it is a good way to look back at the various solutions you come up with. Another thing I need to further improve on is intermittent check-ins.
I recently finished reading Working Effectively with Legacy Code by Michae Feathers and so it was great to be able to put that knowledge to practice. The book makes for a great reference so I actually got the hardcopy as it would be much easier to flip back and forth than the kindle version.
The testing framework already being used by the project is XUnit so it was a chance for me to learn the framework as well. Apart from XUnit, I thought it would be a good opportunity to also start learning how to use fluent assertions.
My Approach
Before making any changes to the original code, I wanted to make sure that I had some tests to verify that the current code is working as described and that I will not breaking anything.
I did have to modify program to be public in order to create an instance of it. I also created a constructor that would assign the Items property on Initialization as by specification, we are not supposed to touch the Item class or Item property.
I also added a GetItem() method to return the items so i can verify that the correct changes were made. Because items is passed by reference, i should just be able to do the assertions on items instead of having to create the local variable changedItems.
Next was a simple conversion of the for loop into a foreach loop to make it easier to extract a method that I can start testing.
Visual studio has some great options for automatic refactoring which you can find when you press Ctrl+. on lines you want to refactor. I basically highlighted everything in the foreach loop and extracted that as a method.
No major changes yet so the test should still run
I then created parameterized tests just for the method so i can easily add more cases. Looking at the spec and looking at the current sample items, it didn’t really test out the extremes like items that are already 50 or 0 and need to be degraded or upgraded in quality. I added a few more test cases so that we exhaust all possibilities. At this point, I left the conjured items to it’s old functionality and chose to deal with it once i have a better view of the code.
At this Point I wanted to extract UpdateItemQuality into its own class. I did the same with the test associated with it as well to make it a little bit more organized.
Now UpdateItemQuality is still a huge and complex method which I didn’t quite want to attack just yet so I made some minor improvements like merging nested if statements using Visual Studio’s automated refactoring just to make sure that I don’t oversimply the statements and end up changing the program every so slightly.
Because I am being non-confrontational, I add the functionality to get a category for an item based on the item name. Again because we want this to be TDD, we create the tests first then develop the methods.
Instead of having the ItemProcessor process all the different items, I wanted to make use of polymorphism so a more specific type of processor worries about the implementation depending on the category so I then created the tests (and corresponding classes for the different processors.
Once I isolated the items into their own update methods, it became fairly easy to refactor the UpdateItemQuality method by just removing the irrelevant cases for each item time.
Because I have my test with exhaustive cases, it gave me the confidence to make all the drastic changes. This also made it easier to then add the new functionality for conjured items once everything was refactored.
The following is a timelapse of one of the iterations of the kata which I felt was fairly presentable 😁