Implementation of the "Gilded Rose" coding kata.
Based on the Ruby ready-to-go version found here.
This document is a stream of consciousness list of thoughts accompanying each commit. The commits will be named in accordance with the sections below.
The complexity in lib/gilded_rose.rb is out of control. The number of branch combinations the code can take makes it impossible to reason about as a whole.
Moreover the tests for conjured items do not pass.
It would be more interesting to think of the assessment of each item as its own process--and then share strategies where possible.
- Mark broken specs as pending, leaving as documentation. Fixing in current implementation is too painful.
- Organize legacy code as standard Ruby.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
- No need to loop in
#update_quality
. Make this responsibility of calling code. (Loops are less composable than single operations.) - Identify inputs and outputs of "quality update" process. Kill mutation of
Item
struct. - Reassign namespace to terser
lib/gr
. - Implement each process.
- Extract shared functionality into common code.
First things first: a random function in the global namespace is garbage.
Let's make the code at least pretend to be Ruby.
- Introduce
GildedRose
namespace. - Move current crappy implementation to this namespace.
- Update references in to this lib spec.
- Remove
should
expectations in spec in favor ofexpect
.
- No need to loop in
#update_quality
. Make this responsibility of calling code. (Loops are less composable than single operations.) - Identify inputs and outputs of "quality update" process. Kill mutation of
Item
struct. - Reassign namespace to terser
lib/gr
. - Implement each process.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
- Extract shared functionality into common code.
First things first: remove the loop from #update_quality
.
Since the only calling code is our spec, we'll make the spec loop over items.
spec/lib/gilded_rose_spec.rb
callsupdate_quality
with one item.- Delete some old specs testing "multiple items", as the functionality is covered already by the sufficiently general existing specs.
- Identify inputs and outputs of "quality update" process.
- Kill mutation of
Item
struct. Return a new struct instead. - Reassign namespace to terser
lib/gr
. - Implement each process.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
- Extract shared functionality into common code.
Looking at our function, two fields are conditionally changed on the provided
Item
:
quality
sell_in
(if item is not "Sulfuras, Hand of Ragnaros")
Since these are ever changed by our function, they could instead be the
outputs. (Rather than changing them on our Item
.) Since mutation causes
unanticipated states, we prefer not doing it.
It appears our implementation needs name
, sell_in
, and quantity
all to
do its work, at least conditionally. (That's good enough--ever is the
criterion.)
The way our implementation gets this information is by querying the provided
Item
. We don't have to do that, though. We could do this:
def update_quality(item)
name, sell_in, quality = *item
...
But then our function still has knowledge of Item
. So let's go a step
further and move those three fields to proper function arguments. This way,
our function can stay happily oblivious to Item
:
def update_quality(name, sell_in, quality)
...
And our outputs (the things ever changed) can be quality
and sell_in
,
returned as an array. Since these two will change, we need to copy them
locally:
def update_quality(name, sell_in, quality)
new_sell_in = sell_in
new_quality = quality
...
Since we're not mutating the provided Item
, we must tweak our specs. Now,
there is no When
--we have a deterministic function that doesn't do anything
dangerous. In fact, we no longer need Item
. So let's delete it.
But now our function is named poorly. (It always was lol.) We're not
"updating" anything. Let's call it #appraise
.
- Update
#update_quality
to take the args it needs rather than a struct. - Cache local
sell_in
andquality
vals, building new ones for return. - Stop mutating
Item
, realize we don't need it right now. - Rename
#update_quality
to#appraise
. - Update specs to call the newly defined function.
- Discover the processes entangled in the method.
- Implement each process. (Maybe multiple commits.)
- Reassign namespace to terser
lib/gr
. - Extract shared functionality into common code.
- Rework specs to reflect these processes.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
Now we need to think a bit harder. What is #appraise
doing? Luckily, because
we made it a deterministic function, we can tell everything we need to know by
its inputs and outputs.
It takes a product's:
- name
- sell_in
- quality
And produces new:
- sell_in
- quality
Name is invariant. OK. But we have two outputs. We know that, AT MOST, each of these outputs needs three pieces of data (the inputs). Let's dig into our implementation and see what each of them needs.
Deriving the "sell_in" output ever requires:
- name
Well, that's not surprising. We appraise "sell_in" for non-legendary items.
Deriving the "quality" output ever requires:
- name
- sell_in
- quality
So, "quality" is way more state dependent than "sell_in". Good to know.
Let's separate the two. Introducing #adjust_sell_in
. It's what we do when
appraising to square away the new "sell_in" for this appraisal.
We should test it, too.
- Separate
adjust_sell_in
function. - Add specs.
- Discover the processes entangled in the method.
- Implement each process. (Maybe multiple commits.)
- Reassign namespace to terser
lib/gr
. - Extract shared functionality into common code.
- Rework specs to reflect these processes.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
Let's go through #appraise
for the "Aged Brie" product, removing one of the
variables, and rewrite it as a separate function #appraise_aged_brie
.
To accomplish this, we:
- Duplicate the code in
#appraise
. - Delete huge chunks of conditional code which will never apply because
name
is invariant. - Notice that
new_sell_in
is never used before its definition, and so make it an argument to this function. - Combine a nested conditional into a new, top-level one.
- Combine two conditionals into a nested one based on
new_quality < 50
. - Note that
adjusted_sell_in
is invariant, so remove it from the results (the calling code already knows the value). - Just cap quality at 50 explicitly, so we don't need branch logic.
- Remove references to "Aged Brie" in original function.
- Update "Aged Brie" specs to use this function instead.
- Code refactored as described above.
- Specs for
#appraise_aged_brie
.
Well, that went well. Let's do it for another product mentioned by name in
#appraise
.
Let's do the backstage passes. We'll follow the same process as before, making
#appraise_backstage_passes
.
- Code refactored as described above.
- Specs for
#appraise_backstage_passes
.
- Discover the processes entangled in the method.
- Implement each process. (Maybe multiple commits.)
- Reassign namespace to terser
lib/gr
. - Extract shared functionality into common code.
- Rework specs to reflect these processes.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
Our #appraise
method is starting to look really anemic. Great!
There's just one more thing that sticks out: the legendary item.
Lather, rinse, repeat.
- Code refactored as described above.
- Specs for
#appraise_backstage_passes
. - Simply remainder of
#appraise
, fix specs.
- Extract shared concerns.
- Add processes for "conjured" items.
- Remove garbage Given/When/Then rspec extension. Just use rspec.
- Generalize code.
- Re-namespace code into
lib/gr
. - Write basic CLI utilizing
lib/gr
.
We now have a collection of bespoke methods for dealing with different items.
Let's flex our brain a little bit to extract commonality of intent out from the code. Generally:
- The "sell_in" attribute of an item with either stay the same or decrease.
- The "quality" attribute can decrease, increase, or assume a constant value.
For both "sell_in" and "quality", changes are applied additively or by assignment.
We can further characterize the items thusly:
- normal - items whose quality depreciates over time.
- fleeting - items whose quality appreciates for a time, then vanishes.
- vintage - items whose quality appreciates indefinitely.
- enduring - items whose quality never changes.
Because of the arbitrariness of the rules for "sell_in" and "quality", we should express effects as heuristics. In Ruby, this is best accomplished with strategies, or subroutines. To keep things simple, we'll have one strategy for each characterization.
We will also reintroduce Item
, since it's no longer an albatross (and is
part of the project requirements).
New #tick
method which accepts an Item
and:
- Gets the next
sell_in
value appropriate for theItem
. - Derives the action to take on an item's quality.
- Applies that action.
- Applies a ceiling to the quality, if applicable.
- Applies a floor to the quality, if applicable.
- Returns a brand new
Item
with the new information.
- Write basic CLI utilizing
lib/gilded_rose
.
Let's write a simple CLI with a starter database of our items in YAML.
- The CLI code.
And that's it. :)