patternMinor
Bayesian approximation method for online ranking
Viewed 0 times
approximationmethodbayesianonlineforranking
Problem
I've implementing the this algorithm in Java, Scala, and Clojure to show my teammates. I know the code works as expected. What I'm looking for is tips on good Clojure style.
```
(ns gaming.online.ranking
"Synopsis:
(bradley-terry-full 5
(list (create-team 1 45 (list (create-player 101 (create-ability-with-stddev 25 8))))
(create-team 2 32 (list (create-player 102 (create-ability-with-stddev 25 8))))))
Updates players' ability compared with how they were expected to perform")
; defn- means a private function
(defn- sum [f xs]
(apply + (map f xs)))
(defn- plus-pair [[a1 a2] [b1 b2]]
[(+ a1 b1) (+ a2 b2)])
(defn split-with-similar
"Where split-with is like [(take-while pred xs) (drop-while pred xs)], this method is like
recursive calls to split-with on the drop-while'd portion
The predicate takes two arguments: the head of the current list, and the element to compare it
against"
[pred xs]
(when xs
; loop/recur is how to do tail recursion in clojure
; 'let', 'do', 'for', 'loop' and function params can destructure collections like this next line
(loop [[head & tail] xs
acc (vector)]
(let [[like-head not-like-head] (split-with #(pred head %) tail)
updated-acc (conj acc (cons head like-head))] ; conj is like cons, but works with vectors too
(if (empty? not-like-head)
acc
(recur not-like-head updated-acc))))))
; defining a class called Ability with these fields. It acts much like a clojure map
(defrecord Ability [mean stddev variance])
; keywords are also functions accepting a map
; maps are also functions accepting a key
; vectors are also functions accepting an index
(defn mean [ability] (:mean ability))
(defn stddev [ability] (:stddev ability))
(defn variance [ability] (:variance ability))
(defn create-ability-with-stddev [mean stddev]
"Create ability from stddev"
(Ability. mean stddev (* stddev stddev))) ; how to call a java constructor
(def
```
(ns gaming.online.ranking
"Synopsis:
(bradley-terry-full 5
(list (create-team 1 45 (list (create-player 101 (create-ability-with-stddev 25 8))))
(create-team 2 32 (list (create-player 102 (create-ability-with-stddev 25 8))))))
Updates players' ability compared with how they were expected to perform")
; defn- means a private function
(defn- sum [f xs]
(apply + (map f xs)))
(defn- plus-pair [[a1 a2] [b1 b2]]
[(+ a1 b1) (+ a2 b2)])
(defn split-with-similar
"Where split-with is like [(take-while pred xs) (drop-while pred xs)], this method is like
recursive calls to split-with on the drop-while'd portion
The predicate takes two arguments: the head of the current list, and the element to compare it
against"
[pred xs]
(when xs
; loop/recur is how to do tail recursion in clojure
; 'let', 'do', 'for', 'loop' and function params can destructure collections like this next line
(loop [[head & tail] xs
acc (vector)]
(let [[like-head not-like-head] (split-with #(pred head %) tail)
updated-acc (conj acc (cons head like-head))] ; conj is like cons, but works with vectors too
(if (empty? not-like-head)
acc
(recur not-like-head updated-acc))))))
; defining a class called Ability with these fields. It acts much like a clojure map
(defrecord Ability [mean stddev variance])
; keywords are also functions accepting a map
; maps are also functions accepting a key
; vectors are also functions accepting an index
(defn mean [ability] (:mean ability))
(defn stddev [ability] (:stddev ability))
(defn variance [ability] (:variance ability))
(defn create-ability-with-stddev [mean stddev]
"Create ability from stddev"
(Ability. mean stddev (* stddev stddev))) ; how to call a java constructor
(def
Solution
This is a huge file to comment on, so maybe just some general tips:
-
-
-
You can use
-
You can use the threading macros
can be written more cleanly as:
-
I would avoid the use of
-
By convention, argument that start with an underscore mean they are ignored, so
-
map and mapv (a variant of map returning a vector) can take any number of sequences as argument, each element of the nth sequence being used as the nth argument of the function given to map. So plus-pair could be written:(defn plus-vectors [v1 v2]
(mapv + v1 v2)-
loop can be replaced by iterate and take-while, but that is mostly a matter of taste-
You can use
map->Ability to instantiate an Ability record-
You can use the threading macros
-> and ->> more often for clarity:(for [opponent teams :when (not= opponent team)] (calc team opponent))can be written more cleanly as:
(->> teams
(remove #{team})
(map calc team))-
I would avoid the use of
:let inside a for definition, I would instead use a proper (let []) inside its body whenever possible-
By convention, argument that start with an underscore mean they are ignored, so
_team might not be the best choice in this exampleCode Snippets
(defn plus-vectors [v1 v2]
(mapv + v1 v2)(for [opponent teams :when (not= opponent team)] (calc team opponent))(->> teams
(remove #{team})
(map calc team))Context
StackExchange Code Review Q#11275, answer score: 5
Revisions (0)
No revisions yet.