Fork me on GitHub
#code-reviews
<
2021-05-18
>
noisesmith12:05:49

the way that code is formatted, it's really hard to tell what it's trying to do - with the font size used the line wraps make the document structure difficult to read in a normal sized browser window. it looks like there are multiple loops in the function body one after another? pasting into a text editor so I can actually read it.

noisesmith12:05:49

the repeated usage of the same loop "template" instead of a function makes this hard to read always use zero? instead of (= 0 ...) always use first instead of (nth ... 0) prefer second to (nth ... 1)

noisesmith12:05:53

prefer ffirst to (nth (first ...) 0)

noisesmith12:05:26

having the arg vector on the same line as the function name is bad because the doc string / metadata would go between the two if present

noisesmith12:05:50

if you used the optional init arg to reduce, that case could just be reduce

noisesmith12:05:17

in the reduce you call + on two list lookups, and send the result to be looked up in a list again. this is an error and once again this is sorted if you use the optional init arg to reduce

noisesmith12:05:49

in fact that whole case can be replaced by the simpler (apply + (map first unimportant)) . + is smart enough to provide 0 for an empty arg list here

noisesmith12:05:00

your custom sorting function just does what a normal sort would have done anyway

user=> (sort [[1 2] [4 5] [3 4]])
([1 2] [3 4] [4 5])

noisesmith12:05:00

each of those loop bodies that are copy-pasted are just doing (apply + (map first ...)) the hard way

noisesmith12:05:08

here's what I end up with:

(defn sum-firsts
  [coll]
  (apply + (map first coll)))

(defn luck-balance
  [k contests]
  (let [unimportant (filter (comp zero? second) contests)
        unimportant-sum (sum-firsts unimportant)
        sorted-important (->> contests
                              (filter #(= 1 (second %)))
                              (sort))
        k-sum (sum-firsts (take k sorted-important))
        remaining-sum (sum-firsts (drop k sorted-important))]
    (prn unimportant-sum
         k-sum
         remaining-sum)
    (- (+ unimportant-sum
          k-sum)
       remaining-sum)))

noisesmith12:05:34

I haven't tested the code, but I think it's closer to what you want than the example you gave

jaihindhreddy15:05:20

Here's my take on it. Perhaps I went overboard with the threading macro:

(defn luck-balance [k contests]
  (let [unimp (->> (filter (comp zero? second) contests)
                   (map first)
                   (apply +))
        imp (->> (filter #(= 1 (second %)) contests)
                  (map first)
                  (sort-by -)
                  (split-at k)
                  (map #(apply + %))
                  (apply -))]
    (+ unimp imp)))
Can be made more efficient by implementing and using a partial sort, as opposed to the sort-by and split-at.