Kata: Gilded Rose in Clojure (II) -> Replacing conditional with polymorphism using multimethods

At the end of the previous post (Clarifying conditional logic), I had this version of the 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

Even though the conditional logic in the update-item-quality function read much better than the original one, I completely got rid of it using the replace conditional with polymorphism refactoring. I used multimethods which is one of the ways of achieving polymorphism in Clojure.

To start this refactoring, I first renamed the update-item-quality function to update-item-quality-old. Then I created a multimethod called update-item-quality with :name as dispatch function and with a method associated to its default dispatch that called the update-item-quality-old function:

	#... code before
	#...
	(defn- update-item-quality-old [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))
	(defmulti update-item-quality :name)
	(defmethod update-item-quality :default [item]
	(update-item-quality-old item))
	#...
	#... code after

view raw introducing-multi.clj hosted with ❤ by GitHub

This new version behaved exactly like the previous one and it was a good base to do the refactoring in small steps (one branch at a time) because every dispatch value not associated yet with a function was calling the old code thanks to the default dispatch.

Next I wrote a defmethod associated to the Age Brie item, copied the code from the Age Brie branch in update-item-quality-old into it and deleted the branch and the aged-brie? query helper. After this change all the tests were still passing.

	# ... code before
	# ...
	(defn- update-item-quality-old [item]
	(cond
	(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))
	(defmulti update-item-quality :name)
	(defmethod update-item-quality :default [item]
	(update-item-quality-old item))
	(defmethod update-item-quality "Aged Brie" [item]
	(increase-quality item 1))
	#...
	#... code after

view raw removed-aged-brie-cond.clj hosted with ❤ by GitHub

Then I continued eliminating the rest of the branches in the cond of the update-item-quality-old function and their associated query functions.

Once I finished wit all the branches, I had replaced the conditional with polymorphism and could safely delete update-item-quality-old. I also changed the method associated with the default dispatch to make it just returned the received item (this was what got executed for the Sulfuras item and it corresponded to the :else branch of the cond in the old code).

To finish the refactoring I extracted the update-regular-item-quality helper to remove some duplication in some defmethods and moved the code that updated the quality into a separate name space, gilded-rose.item-quality:

	(ns gilded-rose.item-quality)
	(defn- update-quality [item value]
	(merge item {:quality value}))
	(defn- increase-quality [{:keys [quality] :as item} times]
	(update-quality item (min 50 (reduce + quality (repeat times 1)))))
	(defn- decrease-quality [{:keys [quality] :as item} times]
	(update-quality item (max 0 (reduce - quality (repeat times 1)))))
	(defn- set-quality-to-zero [{:keys [quality] :as item}]
	(update-quality item 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-regular-item-quality [item]
	(if (after-selling-date? item)
	(decrease-quality item 2)
	(decrease-quality item 1)))
	(defmulti update :name)
	(defmethod update :default [item]
	item)
	(defmethod update "Aged Brie" [item]
	(increase-quality item 1))
	(defmethod update "Backstage passes to a TAFKAL80ETC concert" [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))
	(defmethod update "+5 Dexterity Vest" [item]
	(update-regular-item-quality item))
	(defmethod update "Elixir of the Mongoose" [item]
	(update-regular-item-quality item))

view raw gilded-rose.item-quality.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 Introduced a multimethod which now it's defaulting to the old update item quality function and Added helper to make all items age one day)

After this refactoring I had a nice polymorphic solution that made it easier to add the new conjured items functionality.

The next and last post will show how I modified the multimethod dispatch function to decorate the update quality behavior when it was updating a conjured item.

This is the second 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