Smashing the Gilded Rose

A friendlier type of nuking

Our practice tech test for the past two days at Makers Academy was? Refactoring some hideous legacy code. See that method above? That SINGLE method that is over 40 lines long and bursting with nested if/else conditionals?

Yep, that one. So we had to refactor it AND THEN add an additional item type.

(Ever heard of a squint test? Screw up your eyes and just look at the layout — see all those arrows? A bad sign.)

So this is what I did:

1. Wrote some tests
There was one pre-existing test, which was pretty meaningless anyway. I had no idea what all this code was meant to do, nor did I have any intention of losing myself in a forest of nested conditionals. I read the specs and wrote tests to pass the specs. As luck would have it, the hideous nests actually did work, so eventually I had a full suite of tests and a good knowledge of what the program was designed to do.

2. Deleted everything in the update_quality method
Nuking is not always the best method for refactoring, but that code was just irritating me. Luckily I had tests to guide me so I wasn’t too worried about writing the code.

​
3. Googled ‘ways to refactor nested conditionals’
Came up with references to duck typing

4. Took a nap in the sunshine
Seriously. I was tired at this point and knew I had to have a clear grasp of the design before moving forward. So a nap sounded good.

5. Read POODR’s chapter on duck typing
A lot. Also looked into inheritance but that seemed possibly messy.

A big issue at this particular moment was the instruction that I wasn’t allowed to change the item class. Duck typing was a great solution, but it would require changing the item class, which would apparently upset a goblin.

​​6. Damn the goblin, I’m writing well-designed code
Once I made that decision, it was full steam ahead with TDD’d duck typing. I ran into some snafus about how I was storing and calling objects, but after a few hours had sorted those out and it was really simple. I ended up with several small classes and super-extensible code. I added the new feature on in about half an hour, complete with full testing.

7. Inheritance!
At this point I felt I’d done enough original work to watch Sandi Metz’s video on the Gilded Rose. In it, she mentioned that inheritance isn’t always bad, as long as all of the subclasses share the behaviour of the superclass. So I thought, hey why not DRY this out a bit more and create a Product class which will contain the instantiation behaviour common to all the objects? Easy!

8. Satisfying the goblin
On my way back from the gym later that day, I had an epiphany: I could in fact satisfy the absurd goblin by making the original Item class the superclass and having all the other classes inherit their behaviour! It meant a bit of name-switching (my product and item classes switched names, along with all attendant methods), and the one downside is I couldn’t work out how to throw the errors upon instantiation without changing the item class, so the goblin’s code has slightly less functionality. Still, I’m pretty damn happy with this duck typed, inherited code.

You can check out the full code on my GitHub repo. It should be pretty self-explanatory!

Master branch (with errors for instantiating with disallowed max/min quality)
Item-class branch (item class totally unchanged from original repo)

Don’t hesitate to get in touch if you have more ideas on duck typing, inheritance, or just want to chat code. I’m friendly and I like meeting people!

The refactored method