HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Bayesian approximation method for online ranking

Submitted by: @import:stackexchange-codereview··
0
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

Solution

This is a huge file to comment on, so maybe just some general tips:

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

Code 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.