code-reviews

zendevil 2021-05-18T11:35:16.000300Z

https://prit.substack.com/p/hackerrank-luck-balance

zendevil 2021-05-19T10:32:00.003300Z

updated

jaihindhreddy 2021-05-26T15:53:20.008900Z

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.

2021-05-18T12:29:49.000600Z

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.

2021-05-18T12:32:49.000800Z

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)

2021-05-18T12:34:53.001Z

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

2021-05-18T12:36:26.001200Z

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

2021-05-18T12:37:50.001400Z

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

2021-05-18T12:39:17.001600Z

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

2021-05-18T12:40:49.001800Z

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

2021-05-18T12:43:00.002100Z

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

2021-05-18T12:46:00.002300Z

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

2021-05-18T12:57:08.002700Z

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

2021-05-18T12:57:34.002900Z

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