Thursday, April 30, 2015

Interesting Talk: "Developing ClojureScript With Figwheel"

I've just watched this wonderful talk by Bruce Hauman:

Books I read (January - April 2015)

January
- Software Architecture for Developers, Simon Brown
- Functional Programming Patterns in Scala and Clojure, Michael Bevilacqua-Linn
- Working Effectively with Unit Tests, Jay Fields

February
- Vida y Destino (Жизнь и судьба), Vasili Grossman. (2nd time)
- Primer Libro de Lankhmar (The First Book of Lankhmar), Fritz Leiber
- Drown, Junot Díaz
- Los girasoles ciegos, Alberto Méndez
- Smalltalk Best Practice Patterns, Kent Beck

March
- Las puertas del paraiso (The Vagrants), Yiyun Li
- Growing Object-Oriented Software Guided by Tests, Steve Freeman and Nat Pryce. (2nd time)
- The Joy of Clojure, 2nd edition, Michael Fogus and Chris Houser

April
- Las ciudades carnales (Les cités charnelles), Zoé Oldenbourg
- Refactoring: Improving the Design of Existing Code, Fowler, Beck, Brant, Opdyke and Roberts. (2nd time)
- Hasta aquí hemos llegado (Τίτλοι τέλους), Petros Márkaris
- The Childhood of Jesus, J. M. Coetzee

Interesting Talk: "The Art of the Javascript Metaobject Protocol"

I've jut watched this great talk by Reginald Braithwaite:

Clojure Developers Barcelona: Short talk about sequence comprehensions

Last Tuesday I gave a short talk about sequence comprehensions in Clojure for some of the Clojure Developers Barcelona study group members.

This is the "clean version" of the code I wrote on Lightable to explain it:

;;--------------
;;
;; Sequence comprehensions
;;
;;--------------
;; Clojure generalizes the notion of list comprehension to sequence comprehension
;; Clojure comprehension uses the for macro
;; It takes a vector of binding-form - coll-expr (generator),
;; plus optional filtering expressions
;; and then yields a sequence of expressions
;; (for [binding-form coll-expr filter-expr? ...] expr)
;; 1. Generators
;; 1.1. An example using only one generator
(def three-digits (seq [1 2 3]))
(for [x1 three-digits]
x1)
(def three-letters ["A" "B" "C"])
;; 1.2. An example using two generators
(for [x1 three-letters
x2 three-digits]
[x1 x2])
;;
;; 2. :when clause
;;
;; It filters the elements that are used in the expression
(for [x1 three-digits
x2 three-digits
:when (and (even? x1) (odd? x2))]
[x1 x2 (* x1 x2)])
(defn neighbors [[x-cell y-cell]]
(set (for [x (range (dec x-cell) (+ x-cell 2))
y (range (dec y-cell) (+ y-cell 2))
:when (not (and (= x x-cell) (= y y-cell)))]
[x y])))
(neighbors [0 0])
;;
;; 3. :while clause
;;
;; The evaluation continues while its expression holds true
(def integers (iterate inc 0))
(for [n integers
:while (even? n)] n)
(for [n integers
:while (odd? n)] n)
;;
;; 4. :let allows you to make bindings with derived values
;;
(for [x1 three-digits
x2 three-digits]
(* x1 x2))
(for [x1 three-digits
x2 three-digits
:let [y (* x1 x2)]
:when (> y 5)]
y)
;; Example from Michiel Borkent answer to a question on Stack Overflow (see Reference 2)
;;---------------------------------
;; Although
(for [i (range 10)
:let [x (* i 2)]]
x)
;; is equivalent to:
(for [i (range 10)]
(let [x (* i 2)]
x))
;; you can see the difference when used in combination with :when (or :while):
(for [i (range 10)
:let [x (* i 2)]
:when (> i 5)]
[i x])
(for [i (range 10)]
(let [x (* i 2)]
(when (> i 5) x)))
;;---------------------------------
;;
;; 5. List comprehensions are more general than filter and map, and can in fact emulate them
;;
;; 5.1 Map
(map #(* % %) (range 1 10))
(for [num (range 1 10)]
(* num num))
;; 5.2 Filter (using :when)
(filter odd? (range 1 10))
(for [num (range 1 10)
:when (odd? num)]
num)
;; 5.3 More than map and filter
;(map #(* % %) (filter #(< % 10) integers)) ; <- can't run!
(for [num integers
:while (< num 10)
:when (odd? num)]
(* num num))
;; References
;; 1. Programming Clojure, 2nd edition. Stuart Halloway and Aaron Bedra
;; 2. Use of :let modifier in Clojure, Stack Overflow (http://bit.ly/1Iq2Hld)
;; 3. Project Euler: Problem 4 in Clojure, Leonardo Borges (http://bit.ly/1Et2rPz)
;; 4. for - clojure.core in ClojureDocs (https://clojuredocs.org/clojure.core/for)
view raw for_talk.clj hosted with ❤ by GitHub
As usual I had a great time and learned a lot preparing this talk.

After the talk, we talked about topics we'd like to learn about in the next months and the started to practice in pairs programming the String Calculator kata.

Thanks all for coming!

Saturday, April 18, 2015

Kata: Gilded Rose in Clojure (III) -> Updating conjured items by decoration

After the two previous posts (Clarifying conditional logic and Replacing conditional with polymorphism using multimethods), I had this version of the code:

(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))
that was being used from the gilded-rose.core name space:

(ns gilded-rose.core
(:require [gilded-rose.item-quality :refer [update]]))
(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 (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)))
Then I started introducing the new conjured items functionality using TDD.

These are the new tests for conjured items:

#...
#...
(facts
"Conjured items"
(fact
"Quality decreases by two each day before sell date"
(pass-days
2
[(item "Conjured Elixir of the Mongoose" 17 20)])
=> [(item "Conjured Elixir of the Mongoose" 15 16)])
(fact
"Quality decreases by four each day after sell date"
(pass-days
2
[(item "Conjured Elixir of the Mongoose" 0 20)])
=> [(item "Conjured Elixir of the Mongoose" -2 12)])
(fact
"Quality can't be less than zero"
(pass-days
1
[(item "Conjured Elixir of the Mongoose" 2 1)])
=> [(item "Conjured Elixir of the Mongoose" 1 0)])
(fact
"Conjured Sulfuras is still immutable"
(pass-days
1
[(item "Conjured Sulfuras, Hand of Ragnaros" 0 80)])
=> [(item "Conjured Sulfuras, Hand of Ragnaros" 0 80)])
(fact
"Conjured Aged Brie quality increases by two eachs day before sell date"
(pass-days
2
[(item "Conjured Aged Brie" 2, 0)])
=> [(item "Conjured Aged Brie" 0 4)])
(fact
"Conjured Aged Brie quality also increases by two eachs day after sell date"
(pass-days
2
[(item "Conjured Aged Brie" 0, 0)])
=> [(item "Conjured Aged Brie" -2 4)])
(fact
"Quality can't be greater than 50"
(pass-days
100
[(item "Conjured Aged Brie" 100, 0)])
=> [(item "Conjured Aged Brie" 0 50)])
(fact
"Conjured Backstage Passes quality increases twice faster before sell date"
(pass-days
15
[(item "Conjured Backstage passes to a TAFKAL80ETC concert" 15, 0)])
=> [(item "Conjured Backstage passes to a TAFKAL80ETC concert" 0 50)])
(fact
"Conjured Backstage Passes quality is zero after sell date"
(pass-days
16
[(item "Conjured Backstage passes to a TAFKAL80ETC concert" 15, 0)])
=> [(item "Conjured Backstage passes to a TAFKAL80ETC concert" -1 0)])))
and these is the resulting code of the gilded-rose.item-quality name space:

(ns gilded-rose.item-quality)
(defn- update-quality [item value]
(assoc 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 [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- type-of-item [{name :name}]
(let [item-types-by-name
{"Aged Brie" :aged-brie
"Backstage passes to a TAFKAL80ETC concert" :backstage-pass
"+5 Dexterity Vest" :regular-item
"Elixir of the Mongoose" :regular-item}]
(if (.contains name "Conjured")
:conjured
(item-types-by-name name))))
(defmulti update type-of-item)
(defmethod update :conjured [{name :name :as item}]
(let
[not-conjured-item-name (clojure.string/replace name #"Conjured " "")
not-conjured-item (assoc item :name not-conjured-item-name)]
(assoc (update (update not-conjured-item))
:name name)))
(defmethod update :default [item]
item)
(defmethod update :aged-brie [item]
(increase-quality item 1))
(defmethod update :backstage-pass [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 :regular-item [item]
(if (after-selling-date? item)
(decrease-quality item 2)
(decrease-quality item 1)))
Notice the change in the update multimethod dispatch function. Now instead of being :name as before, it's the function type-of-item that returns the dispatch value :conjured if the item is a conjured one (i. e., if its name contains "Conjured"), or the type of item corresponding to each item name otherwise (which it looks up in the item-types-by-name map).

I also added a defmethod for the :conjured dispatch values which decorates update by calling it twice passing the not conjured version of the item and modified the other defmethod functions to use the type of item instead of its name. This made possible a better way of removing the duplication for regular items than the previous update-regular-item-quality private function.

This simple decoration made all the tests shown before pass, except the "Conjured Sulfuras is still immutable" one. For this test to pass I had to modify the degradable-item? query in the gilded-rose.core name space:

(ns gilded-rose.core
(:require [gilded-rose.item-quality :refer [update]]))
(defn- degradable-item? [{name :name}]
(not (.contains name "Sulfuras, Hand of Ragnaros")))
(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 (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)))
That's all. 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 from Conjured items quality decreases by two each day before sell date on)

Starting from the polymorphic version of update, we had got through refactoring, made it easy to add the new conjured items functionality as a decoration of update.

Compare this Clojure version of Gilded Rose with the Java version I did some time ago.

This is the last post in this series about the Gilded Rose kata in Clojure:
  1. Clarifying conditional logic
  2. Replacing conditional with polymorphism using multimethods
  3. Updating conjured items by decoration

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)))
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
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
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))
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

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)))
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)])))
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)))
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

Saturday, April 4, 2015

Moving the date range code into an Angular directive

At the end of last post, we had eliminated some duplication from an Angular controller by using events to make it communicate with some of the widgets it was using. This was the code at that moment:

"use strict";
app.controller('TracksIndexPageController', [
'$scope',
'Categories',
'accumulatedStatistics',
'categoryListWidgetFactory',
'dateRangeWidgetFactory',
'Pagination',
'activitiesWidgetFactory',
'applicationEvents',
function ($scope, Categories, accumulatedStatistics, categoryListWidgetFactory, dateRangeWidgetFactory, Pagination, activitiesWidgetFactory, applicationEvents) {
var updatePageInfo = function () {
$scope.activities.filterBy($scope.categoryList.getSelectedCategory());
$scope.updateActivitiesRelatedData();
$scope.pagination = new Pagination($scope.activities.displayedOnes);
},
updateStatistics = function () {
$scope.displayedStatistics = accumulatedStatistics.getGroupedStatisticsToDisplay(
$scope.activities.displayedOnes
);
};
$scope.$on(
applicationEvents.DateRange_Changed,
function () {
$scope.activities.load($scope.dateRange, updatePageInfo);
}
);
$scope.$on(
applicationEvents.CategoryList_NewSelectedCategory,
updatePageInfo
);
$scope.updateActivitiesRelatedData = function () {
updateStatistics();
$scope.categoryList.updateNonEmptyCategories(
$scope.activities.distinctCategories()
);
};
$scope.userHasStatistics = function () {
return accumulatedStatistics.exists();
};
$scope.shouldShowActivities = function () {
return !resolved() || $scope.activities.thereAreSomeToDisplay();
function resolved() {
return _.isUndefined($scope.activities.displayedOnes.$resolved) ||
$scope.activities.displayedOnes.$resolved;
}
};
$scope.initialize = function () {
$scope.Categories = Categories;
$scope.categories = Categories.descriptions;
$scope.activities = activitiesWidgetFactory.create();
$scope.displayedStatistics = [];
$scope.categoryList = categoryListWidgetFactory.create(
Categories.allCategories,
$scope.activities.distinctCategories()
);
$scope.dateRange = dateRangeWidgetFactory.create();
$scope.dateRange.today();
};
$scope.initialize();
}
]);
Next we got advantage of the decoupling given by using events to move the rest of the code that had to do with the date range widget to an Angular directive.

First we created the directive:

"use strict";
app.directive('dateRange', [
'dateRangeWidgetFactory',
function(dateRangeWidgetFactory) {
return {
restrict: 'AE',
templateUrl: '/app/components/tracks/list/dateRangeWidget/date-range.html',
controller: function ($scope) {
$scope.dateRange = dateRangeWidgetFactory.create();
$scope.dateRange.today();
}
};
}
]);
where we moved the code that creates and initializes the date range widget.

Then we moved all the html that was using the date range widget in track-index.html to the date-range.html template:

<div class="filter-ui__wrapper">
<div class="filter-ui__box date-nav">
<button class="is--left filter-arrows svg-arrow-left-bold"
ng-click="dateRange.previous()">
<span>&lt;</span>
</button>
<button ng-click="dateRange.today()">{{ 'tracks.today' | i18n }}</button>
<button class="is--right filter-arrows svg-arrow-right-bold"
ng-click="dateRange.next()">
<span>&gt;</span>
</button>
</div>
<div class="date-data">
<span>{{ dateRange.fromDate() | date: 'dd/MM/yyyy' }}</span>
-
<span>{{ dateRange.toDate() | date: 'dd/MM/yyyy' }}</span>
</div>
<div class="filter-ui__box date-periods">
<button class="is--left"
ng-class="{ 'is--active': dateRange.isUsingWeekPeriod() }"
ng-click="dateRange.useWeekPeriod()">
{{ 'tracks.week' | i18n }}
</button>
<button ng-class="{ 'is--active': dateRange.isUsingMonthPeriod() }"
ng-click="dateRange.useMonthPeriod()">
{{ 'tracks.month' | i18n }}
</button>
<button class="is--right"
ng-class="{ 'is--active': dateRange.isUsingYearPeriod() }"
ng-click="dateRange.useYearPeriod()">
{{ 'tracks.year' | i18n }}
</button>
</div>
</div>
view raw date-range.html hosted with ❤ by GitHub
In track-index all this code got substituted by:

<!--...-->
<!--some more code before-->
<date-range></date-range>
<!--some more code after-->
<!--...-->
This is how the controller's code looks now:

"use strict";
app.controller('TracksIndexPageController', [
'$scope',
'Categories',
'accumulatedStatistics',
'categoryListWidgetFactory',
'Pagination',
'activitiesWidgetFactory',
'applicationEvents',
function ($scope, Categories, accumulatedStatistics, categoryListWidgetFactory, Pagination, activitiesWidgetFactory, applicationEvents) {
var updatePageInfo = function () {
$scope.activities.filterBy($scope.categoryList.getSelectedCategory());
$scope.updateActivitiesRelatedData();
$scope.pagination = new Pagination($scope.activities.displayedOnes);
},
updateStatistics = function () {
$scope.displayedStatistics = accumulatedStatistics.getGroupedStatisticsToDisplay(
$scope.activities.displayedOnes
);
};
$scope.$on(
applicationEvents.DateRange_Changed,
function () {
$scope.activities.load($scope.dateRange, updatePageInfo);
}
);
$scope.$on(
applicationEvents.CategoryList_NewSelectedCategory,
updatePageInfo
);
$scope.updateActivitiesRelatedData = function () {
updateStatistics();
$scope.categoryList.updateNonEmptyCategories(
$scope.activities.distinctCategories()
);
};
$scope.userHasStatistics = function () {
return accumulatedStatistics.exists();
};
$scope.shouldShowActivities = function () {
return !resolved() || $scope.activities.thereAreSomeToDisplay();
function resolved() {
return _.isUndefined($scope.activities.displayedOnes.$resolved) ||
$scope.activities.displayedOnes.$resolved;
}
};
$scope.initialize = function () {
$scope.Categories = Categories;
$scope.categories = Categories.descriptions;
$scope.activities = activitiesWidgetFactory.create();
$scope.displayedStatistics = [];
$scope.categoryList = categoryListWidgetFactory.create(
Categories.allCategories,
$scope.activities.distinctCategories()
);
};
$scope.initialize();
}
]);
Notice how the dateRangeWidgetFactory is not being injected into the controller any more and the widget creation and initilization code is gone.

We should have probably used a similar design from the very beginning (with a directive, a widget and communicating through events) but we didn't, because we were in a hurry, still learning how the page design was meant to be and still learning a way to avoid repeating past errors in Angular applications.

Compare the current version of the code with how it looked at the beginning (it was much longer, I'm only showing the date range related code):

"use strict";
app.controller('TracksIndexPageController', [
'$scope',
'$filter',
'$http',
'Track',
'currentUser',
'Categories',
'AccumulatedStatistics',
'TrackFilteringOptions',
'CategoryListWidget',
'CategoriesFilteringOptions',
function ($scope, $filter, $http, Track, currentUser, Categories, AccumulatedStatistics, TrackFilteringOptions, CategoryListWidget, CategoriesFilteringOptions) {
$scope.apikey = currentUser.apikey;
// TODO: Move to constants?
$scope.WEEKLY_PERIOD = "week";
$scope.MONTHLY_PERIOD = "month";
$scope.YEARLY_PERIOD = "year";
$scope.currentRangeStartDate= new Date();
$scope.period = $scope.MONTHLY_PERIOD;
//...
//some code not shown
//...
$scope.updateRange = function(direction) {
var now = moment($scope.currentRangeStartDate);
if ($scope.period == $scope.WEEKLY_PERIOD) {
var newWeek = now.isoWeek() + direction;
$scope.currentRangeStartDate = moment().isoWeek(newWeek).isoWeekday(1).toDate();
$scope.displayFromDate = moment().isoWeek(newWeek).isoWeekday(1).toDate();
$scope.displayToDate = moment().isoWeek(newWeek).isoWeekday(7).toDate();
}
if ($scope.period == $scope.MONTHLY_PERIOD) {
var newMonth = now.month() + direction;
$scope.currentRangeStartDate = moment().month(newMonth).date(1).toDate();
$scope.displayFromDate = $scope.currentRangeStartDate;
$scope.displayToDate = moment().month(newMonth).add('months', 1).date(0).toDate();
}
if ($scope.period == $scope.YEARLY_PERIOD) {
var newYear = now.year() + direction;
$scope.currentRangeStartDate = moment().year(newYear).dayOfYear(1).toDate();
$scope.displayFromDate = $scope.currentRangeStartDate;
$scope.displayToDate = moment().year(newYear).add('year', 1).dayOfYear(0).toDate();
}
$scope.filterData();
};
//...
//some code not shown
//...
$scope.setRangeSizeTo = function(period) {
$scope.period = period;
$scope.updateRange( 0);
return false;
};
$scope.today = function() {
$scope.currentRangeStartDate = new Date();
return false;
};
$scope.nextRange = function() {
$scope.updateRange( 1);
return false;
};
$scope.previousRange = function() {
$scope.updateRange(-1);
return false;
};
//...
//some code not shown
//...
$scope.updateRange(0);
}
]);
The date range logic in this initial version had no tests at all and in fact it had some bugs...

Then we started working on it by extracting its code to a plain JavaScript object and testing it (as we described for a different case here), eliminating some remaining duplication in the controller making it work with events (described in here) and finally created an Angular directive for it.

I think we did something similar to what Liz Keogh writes in her post If you can’t write tests first, at least write tests second:
  • We didn't know how to test first this code and we were under a situation of flux, high uncertainty and time pressure, so we wrote a messy code that mostly worked. Then from there, we put it under a test harness and started improving its design.