5
\$\begingroup\$

I'm learning Clojure and am a rank n00b at it, trying to learn from books and online tutorials (but I'm sometimes concerned that I am picking up bad habits or at least not all the good habits). For exercise I did the Breaking the Records problem on Hackerrank.

TL;DR problem description:

For a list of scores (in historic order), count the number of times the previous best score was exceeded as well as the previous worst score was undercut.

In an iterative language it is quite easy to just iterate through the list; but Clojure does not do iteration and I decided to tackle this problem for exercise in constructing a (tail end) recursive solution. To make it easier for myself, I first did a recursive solution in Java, which I then translated. All in all, it's quite a simple recursive function without rocket surgery involved.

Obviously my code works, as shown by the included unit tests. My concerns are however as follows:

  • When put next to the Java code, the two look fairly similar. Did I follow "idiomatic" Clojure programming, or is it just a clumsy "word-for-word transliteration"?
  • Are there any areas that could have been more compact and/or easier to understand by using different Clojure constructs?

Your critical input will be highly appreciated - including regarding the unit testing, as that is arguably an important part of programming, which I want to learn and practice in parallel.

Code:

(ns hackerrank.breaking-records
  (:require [clojure.test :refer :all]))

(defrecord Record [min max countworse countbetter])

(defn recalc-record [rec newscore]
  (Record.
    (min newscore (:min rec))
    (max newscore (:max rec))
    (+ (:countworse rec) (if (> (:min rec) newscore) 1  0))
    (+ (:countbetter rec) (if (< (:max rec) newscore) 1  0))))

(defn accumulate [curr-record remaining-scores]
  (if (nil? (second remaining-scores))
    curr-record
    (recur (recalc-record curr-record (second remaining-scores)) (rest remaining-scores)))
)

(defn breaking-records [scores]
  (let [result (accumulate (Record. (first scores) (first scores) 0 0) scores)]
    (list (:countbetter result) (:countworse result))))

(deftest test-records
  (testing "edge cases"
    (is (= '(0 0) (breaking-records '())) "no games played yet")
    (is (= '(0 0) (breaking-records '(5))) "single game"))
  (testing "hackerrank examples"
    (is (= '(2 4) (breaking-records '(10 5 20 20 4 5 2 25 1))))
    (is (= '(4 0) (breaking-records '(3 4 21 36 10 28 35 5 24 42)))))
)
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I re-wrote your solution to use more typical Clojure features. When you are looping over data and need to keep track of accumulated state, it is hard to beat loop/recur. A first example:

(ns tst.demo.core
  (:use clojure.test))

(defn breaking-records
  [scores]
  ; this loop has 5 variables. Init all of them
  (loop [low         (first scores)
         high        (first scores)
         nworse      0
         nbetter     0
         score-pairs (partition 2 1 scores)]
    (if (empty? score-pairs)
      {:nbetter nbetter :nworse nworse}
      (let [curr-score-pair (first score-pairs)
            new-score       (second curr-score-pair)]
        ; start the next iteration with modified versions of the 5 loop vars
        (recur
          (min new-score low)
          (max new-score high)
          (if (< new-score low)
            (inc nworse)
            nworse)
          (if (< high new-score)
            (inc nbetter)
            nbetter)
          (rest score-pairs))))))

and unit tests:

(deftest test-records
  (testing "edge cases"
    (is (= (breaking-records []) {:nbetter 0 :nworse 0}) "no games played yet")
    (is (= (breaking-records [5]) {:nbetter 0 :nworse 0}) "single game"))
  (testing "hackerrank examples"
    (is (= (breaking-records [10 5 20 20 4 5 2 25 1]) {:nbetter 2 :nworse 4}))
    (is (= (breaking-records [3 4 21 36 10 28 35 5 24 42]) {:nbetter 4 :nworse 0}))))

; ***** NOTE: it's much easier to use vectors like [1 2 3] instead of a quoted list `(1 2 3)

Please see this list of documentation, esp. the Clojure CheatSheet. Also, the template project as a whole shows how I like to structure things. :)

The function that helps the most is partition. See the docs.


Slight refactoring

You can simplify it a small amount and make it a bit more compact by using more specialized functions like reduce and cond->. This version uses a map to hold state and reduce to perform the looping:

(defn breaking-records
  [scores]
  (let [state-init     {:low     (first scores)
                        :high    (first scores)
                        :nworse  0
                        :nbetter 0}
        accum-stats-fn (fn [state score-pair]
                         ; Use map destructuring to pull out the 4 state variables
                         (let [{:keys [low high nworse nbetter]} state 
                               new-score (second score-pair)
                               state-new {:low     (min new-score low)
                                          :high    (max new-score high)
                                          :nworse  (cond-> nworse
                                                     (< new-score low) (inc))
                                          :nbetter (cond-> nbetter
                                                     (< high new-score) (inc))}]
                           state-new))
        state-final    (reduce accum-stats-fn
                         state-init
                         (partition 2 1 scores))
        result         (select-keys state-final [:nworse :nbetter])]
    result))
\$\endgroup\$
2
  • \$\begingroup\$ I highly appreciate you taking the time to do this, it does point me to some areas I should be focusing on for learning. It does seem you have not bought into the "small functions" exhortations of the Clean Code book/ideology :-) (To be honest, I have started to get second thoughts about that bit too. Ironically, Robert C. Martin's enthusiasm regarding Clojure was a contributing factor for my interest in it.) Also, is there a specific reason for [] being "much easier" than '(), apart from the one extra character and it standing out from the surrounding parentheses? \$\endgroup\$
    – frIT
    Commented Oct 26, 2020 at 15:08
  • 1
    \$\begingroup\$ When you print a quoted list, the quote disappears, so you always have to add it back when you cut/paste. Same for double-quotes on a string, which is why keywords are nicer for non-user data (e.g. :nworse). \$\endgroup\$ Commented Oct 26, 2020 at 15:14

Not the answer you're looking for? Browse other questions tagged or ask your own question.