Originally taken from: https://github.com/emilybache/GildedRose-Refactoring-Kata. I also used the book Refactoring as reference.
Summing it up: Refactoring code - so it's maintainable again. The starting code sample is exemplary for legacy code. A lot of nested branches, duplicated code, complicated logic and no tests.
For the requirements of this challenge, have a look here
Here are the steps I took. I tried to make them as small as possible - see the git history for detailed reference.
- Starting point - clone repo and throw anything out except the rust part
- Writing the test suite in BDD Style (given-when-then).
- Change Sulfaras and Backstage passes to the Strings defined in the requirements
- Start with the most intended (most nested) parts of the code
- Change Conditions
- Inverse Conditions
- Make else the "default" branch for normal items
- Remove Conditions by min or max clipping the quality value
- Merge branches, so the conditions, e.g.
Backstage passes
is not checked twice
- Separate immutable values from modifications. Rather Compute the quality value/increment, and assign the computed value in the end: Calculate the quality increment
- Move assignments out the branches to top level
- Move queries to top level
- Extract methods on calculations in between branch-bodies (if-else)
- Also calculate the sell-in value: Out of symmetry to the quality value
- Refactor into static function
- Instead of the increment, calculate quality directly
- Remove top level Condition for
Sulfaras
. Different abstraction level- By moving the clipping (min/max) to the new
calculate_quality
method
- By moving the clipping (min/max) to the new
- For symmetry, also calculate sell_directly instead of its increment
- Instead of
if-else
branching, use polymorphism: Therefore move logic into classes
- Do the same with the
sell_in
calculation. But use default behaviour on a trait, as onlySulfaras
differs.
- Move increment methods from gilded to the new classes, because of different level ob abstraction
- Move shared increment logic to its own class, so the
GildedRose
class is free of the "how to increment" logic
The refactoring is fine for me, of course you can still refactor. But now adding the Conjured
items seems to be
pretty straight forward
What do I need to implement for it?:
- Factory returning a new class
- Implement both needed traits:
CalculateQuality
andCalculateSellIn
- Calculate the increment as delegate: But we can re-use everything.
Done:
- Write test suite for conjured cookie
- Implement as stated above
- Write tests first
- Even if the code exists already
- Make the tests fail first, therefore temporarily change the production code, so you know the test actually tests the part of the code you actually want to test :)
- Make small refactoring steps, then test. If the tests fail, undo your changes. No need for debugging.
- Change things when needed. If a prior refactoring does not fit anymore - change it to your current needs (e.g. a name for function does not fit anymore after two commits, change it)
- Tags v0.0.10 and v.0.0.11 do not exist anymore, as they got amended into the v0.0.9