A few days ago I linked to the talk All The Little Things by Sandi Metz, which is about about creating small objects while programming, using the Gilded Rose kata as an example.
It makes heavy use of refactoring under test driven development (TDD), of which more later.
Of course, it infected me too, and I had to have a look at it.
First, I'll admit, having seen Sandi's talk, I am completely indebted to her for having seen through to the core of the problem, and outlining a strategy of isolating the special cases as being the key to solving it.
Also, while I don't identify as being boolean-impaired, I still find the original code a little intimidating. So, I decided to try and do my refactoring in as stupid a manner as possible, learning only as much as I needed to as I went, and only trying to understand the labrynthine conditionals when it became necessary.
So, I grabbed a copy of the kata from the internet.
( Original codeCollapse )
Now, because "Aged Brie" is the first case mentioned in the code, that's the one I'm going to factor out first. The way I'm going to do that is to use an "if" to create one code path for "Aged Brie", and the "else" for everything else, and then I'm going to put all the code in both halves of the if statement:
( Double troubleCollapse )
OK, that's a lot of code. The thing is, now I can simplify the first half because I know that items[i].name is always "Aged Brie", and I can simplify the second half because I know that items[i].name is never "Aged Brie". So there are places I can cut clauses out of "if" statements, places I can remove the "if" statement leaving just the body, places I can delete entire "else" clauses, et cetera...
( Mmmmm.....brieCollapse )
Not bad. Now I can do the same thing with "Backstage passes". Create a new clause in the "if" chain, put all the code from the "else" statement in there, and then simplify both pieces:
( Backstage passes!Collapse )
From there, even for those who are boolean-impaired, it's not that hard to figure out that no changes ever happen for "Sulfuras". Still, I'll add a new clause, copy the code and simplify as before, to get the "final" version of the code for this post:
( Code CompleteCollapse )
Note that I did all that without really needing to understand what those massively nested "if" statements were doing, or even understanding what any of the tests do.
I just had to copy code, reduce some "if" clauses to "true" or "false", and then cut away code which was obviously useless or unreachable. If I had tests (the version I found didn't have any, unlike Sandi's) then they would be useful to ensure I hadn't made a stupid mistake. But I didn't need them as a guide to writing any new code.
Obviously, there's more work that could be done. The "x = x - 1" can be made more readable by transforming it to "x -= 1", some of the nested "if"s can be simplifed by using "&&", and if you wanted you could move the body of each case into its own function (or object). But right now the code is tractable, and adding "Conjured" is a case of following the new pattern.
And if that were it, my curiosity would have been satisfied, and you wouldn't be reading this.
( But...Collapse )
It's normally a mistake to rewrite from scratch, unless you really understand what your code is doing, and why. Even if you have good tests. Good tests can be a massive help, but they can't save you every time.