Kata: Gilded Rose in Clojure (I) -> Clarifying conditional logic

Yesterday I did the Gilded Rose refactoring kata in Clojure which I started working on in the last Clojure Developers Barcelona meetup.

This is the original code that I got from a Mike Jansen's GitHub repository:

	(ns gilded-rose.core)
	(defn update-quality [items]
	(map
	(fn[item] (cond
	(and (< (:sell-in item) 0) (= "Backstage passes to a TAFKAL80ETC concert" (:name item)))
	(merge item {:quality 0})
	(or (= (:name item) "Aged Brie") (= (:name item) "Backstage passes to a TAFKAL80ETC concert"))
	(if (and (= (:name item) "Backstage passes to a TAFKAL80ETC concert") (>= (:sell-in item) 5) (< (:sell-in item) 10))
	(merge item {:quality (inc (inc (:quality item)))})
	(if (and (= (:name item) "Backstage passes to a TAFKAL80ETC concert") (>= (:sell-in item) 0) (< (:sell-in item) 5))
	(merge item {:quality (inc (inc (inc (:quality item))))})
	(if (< (:quality item) 50)
	(merge item {:quality (inc (:quality item))})
	item)))
	(< (:sell-in item) 0)
	(if (= "Backstage passes to a TAFKAL80ETC concert" (:name item))
	(merge item {:quality 0})
	(if (or (= "+5 Dexterity Vest" (:name item)) (= "Elixir of the Mongoose" (:name item)))
	(merge item {:quality (- (:quality item) 2)})
	item))
	(or (= "+5 Dexterity Vest" (:name item)) (= "Elixir of the Mongoose" (:name item)))
	(merge item {:quality (dec (:quality item))})
	:else item))
	(map (fn [item]
	(if (not= "Sulfuras, Hand of Ragnaros" (:name item))
	(merge item {:sell-in (dec (:sell-in item))})
	item))
	items)))
	(defn item [item-name, sell-in, quality]
	{:name item-name, :sell-in sell-in, :quality quality})
	(defn update-current-inventory[]
	(let [inventory
	[
	(item "+5 Dexterity Vest" 10 20)
	(item "Aged Brie" 2 0)
	(item "Elixir of the Mongoose" 5 7)
	(item "Sulfuras, Hand of Ragnaros" 0 80)
	(item "Backstage passes to a TAFKAL80ETC concert" 15 20)
	]]
	(update-quality inventory)))

view raw gilded-rose-original.clj hosted with ❤ by GitHub

First of all I wrote tests following the description of the kata and had to fix several bugs related with items quality inferior and superior limits, 0 and 50 respectively.

These are the tests using Midje:

	(ns gilded-rose.core-test
	(:use midje.sweet)
	(:use [gilded-rose.core]))
	(defn pass-days [n inventory]
	(nth (iterate update-quality inventory) n))
	(facts
	"about gilded rose"
	(facts
	"Sulfuras"
	(pass-days
	5
	[(item "Sulfuras, Hand of Ragnaros" 0 80)]) => [(item "Sulfuras, Hand of Ragnaros" 0 80)])
	(facts
	"Regular items"
	(facts
	"Quality decreases one by one before selling date"
	(pass-days
	5
	[(item "+5 Dexterity Vest" 10 20)]) => [(item "+5 Dexterity Vest" 5 15)]
	(pass-days
	5
	[(item "Elixir of the Mongoose" 5 7)]) => [(item "Elixir of the Mongoose" 0 2)])
	(fact
	"Once the sell by date has passed, quality degrades twice as fast"
	(pass-days
	10
	[(item "+5 Dexterity Vest" 0 20)]) => [(item "+5 Dexterity Vest" -10 0)])
	(fact
	"The quality of an item is never negative"
	(pass-days
	20
	[(item "+5 Dexterity Vest" 10 20)]) => [(item "+5 Dexterity Vest" -10 0)]))
	(facts
	"Aged Brie"
	(fact
	"Quality increases by one before sell date"
	(pass-days
	2
	[(item "Aged Brie" 2 0)]) => [(item "Aged Brie" 0 2)])
	(fact
	"Quality also increases by one after sell date"
	(pass-days
	4
	[(item "Aged Brie" 2 0)]) => [(item "Aged Brie" -2 4)])
	(fact
	"Quality can't be greater than 50"
	(pass-days
	51
	[(item "Aged Brie" 2 0)]) => [(item "Aged Brie" -49 50)]))
	(facts
	"Backstage passages"
	(fact
	"Before 10 days of the concert quality increases one by one"
	(pass-days
	5
	[(item "Backstage passes to a TAFKAL80ETC concert" 15 20)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" 10 25)])
	(fact
	"Between 10 and 5 days of the concert quality increases two by two"
	(pass-days
	5
	[(item "Backstage passes to a TAFKAL80ETC concert" 10 20)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" 5 30)])
	(fact
	"The last 5 days before the concert quality increases three by three"
	(pass-days
	5
	[(item "Backstage passes to a TAFKAL80ETC concert" 5 20)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" 0 35)])
	(fact
	"After the concert quality is 0"
	(pass-days
	6
	[(item "Backstage passes to a TAFKAL80ETC concert" 5 20)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" -1 0)])
	(fact
	"Quality can't be grater than 50"
	(pass-days
	2
	[(item "Backstage passes to a TAFKAL80ETC concert" 10 48)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" 8 50)]
	(pass-days
	17
	[(item "Backstage passes to a TAFKAL80ETC concert" 17 20)])
	=> [(item "Backstage passes to a TAFKAL80ETC concert" 0 50)])))

view raw guilded-rose-tests.clj hosted with ❤ by GitHub

Once all the tests were in place, I started the refactoring by improving the indentation and using destructuring on the item parameter to eliminate duplication and make the code read better. Then I started to work on the cond branches working my way to have a separated branch for each type of item.

Every time I separated a branch for a type of item, I created a query helper to make the branch predicate more readable.

Once I had a branch for each type of item, I created helpers to increase, decrease and set to zero the quality of an item.

Finally, I removed the destructuring of item that I had introduced at the beginning, since it wasn't necessary any more and created some other helper functions.

This is the resulting code:

	(ns gilded-rose.core)
	(defn- regular? [{name :name}]
	(or (= "+5 Dexterity Vest" name)
	(= "Elixir of the Mongoose" name)))
	(defn- aged-brie? [{name :name}]
	(= name "Aged Brie"))
	(defn- backstage-passes? [{name :name}]
	(= name "Backstage passes to a TAFKAL80ETC concert"))
	(defn- increase-quality [{:keys [quality] :as item} times]
	(merge item
	{:quality (min 50 (reduce + quality (repeat times 1)))}))
	(defn- decrease-quality [{:keys [quality] :as item} times]
	(merge item
	{:quality (max 0 (reduce - quality (repeat times 1)))}))
	(defn- set-quality-to-zero [{:keys [quality] :as item}]
	(merge item {:quality 0}))
	(defn- after-selling-date? [{sell-in :sell-in}]
	(< sell-in 0))
	(defn- ten-or-more-days-to-selling-date? [{sell-in :sell-in}]
	(>= sell-in 10))
	(defn- between-days-to-selling-date? [lower higher {sell-in :sell-in}]
	(and (>= sell-in lower) (< sell-in higher)))
	(defn- update-item-quality [item]
	(cond
	(aged-brie? item) (increase-quality item 1)
	(backstage-passes? item)
	(cond
	(ten-or-more-days-to-selling-date? item) (increase-quality item 1)
	(between-days-to-selling-date? 5 10 item) (increase-quality item 2)
	(between-days-to-selling-date? 0 5 item) (increase-quality item 3)
	(after-selling-date? item) (set-quality-to-zero item)
	:else item)
	(regular? item)
	(if (after-selling-date? item)
	(decrease-quality item 2)
	(decrease-quality item 1))
	:else item))
	(defn- degradable-item? [{name :name}]
	(not= "Sulfuras, Hand of Ragnaros" name))
	(defn- age-one-day [{sell-in :sell-in :as item}]
	(merge item {:sell-in (dec sell-in)}))
	(def ^:private all-age-one-day
	(partial map #(if (degradable-item? %) (age-one-day %) %)))
	(defn update-quality [items]
	(map update-item-quality
	(all-age-one-day items)))
	(defn item [item-name, sell-in, quality]
	{:name item-name, :sell-in sell-in, :quality quality})
	(defn update-current-inventory[]
	(let [inventory
	[(item "+5 Dexterity Vest" 10 20)
	(item "Aged Brie" 2 0)
	(item "Elixir of the Mongoose" 5 7)
	(item "Sulfuras, Hand of Ragnaros" 0 80)
	(item "Backstage passes to a TAFKAL80ETC concert" 15 20)]]
	(update-quality inventory)))

view raw guilded-rose.clj hosted with ❤ by GitHub

You can follow the whole process I've just described having a look at the commits I did after every small refactoring (look at commits between Gilded Rose original code and Introduced explaining helper)

This version of update-item-quality is much more readable that the original one.

In the next post, I'll replace the conditional logic with polymorphism using Clojure multimethods.

This is the first post in a series of posts about the Gilded Rose kata in Clojure:

1. Clarifying conditional logic
2. Replacing conditional with polymorphism using multimethods
3. Updating conjured items by decoration