sorted-map-by bug

[Kovas Palunas]

Hey all,

I think I've found a bug in `sorted-map-by`, as shown by the below code:

```

(ns app.core-test
  (:require
   [cljs.test :refer (deftest is)]
   [clojure.string :as st]))

(prn *clojurescript-version*)  ;; "1.10.914"

(defn single-row [result-row]
  {:input (:input result-row)
   (keyword (st/join [(name (:biomarker result-row)) "-datapoints"])) (:datapoints result-row)})

(defn sort-map [m]
  ; Sort row so that :input is first, then put this rest in alphabetical order
  (into (sorted-map-by #(if (= % :input) "aaaaa" (name %))) m))

(defn get-per-input-row [same-input-results]
  ; Calling sort-map twice here resolves the problem.
  (sort-map (reduce merge (map single-row same-input-results))))

(defn make-per-input-results
  [results]
  (let [rows-by-input (group-by :input results)]
    (map get-per-input-row (vals rows-by-input))))

(deftest test-bad-map-sorting
  (is (= '({:input 1 :a-datapoints 5}
           {:input 2 :a-datapoints 5}
           {:input 3 :b-datapoints 5}
           {:input 4 :b-datapoints 5})
         (make-per-input-results
          [{:input 1 :biomarker :a :datapoints 5}
           {:input 2 :biomarker :a :datapoints 5}
           {:input 3 :biomarker :b :datapoints 5}
           {:input 4 :biomarker :b :datapoints 5}]))))

```

The testcase fails, when I would expect it to succeed.  I tried posting this on the cljs JIRA as a bug, but I don't have permission, so I thought I'd post it here instead.  Anyone want to try reproducing this and posting on the JIRA if you can reproduce it?

 - Kovas

[Lauri Pesonen]

Hi Kovas,

When I run your code in a Clojure repl (as opposed to CLJS), I get an exception:

clojure.lang.ArityException: Wrong number of args (2) passed to: app.core-test/sort-map/fn--24334

And this is because you're the comparator that you pass to sorted-map-by is a 1-arity function: #(if (= % :input) "aaaaa" (name %)))

The comparator will be called with two map entries as arguments so that the comparator can decide which one of them should be first, so your comparator function should do something with %1 and %2. But I can't really tell from the code what you're trying to do, so I'm not sure what the comparator should look like.

Cheers,

Lauri

--
Note that posts from new members are moderated - please be patient with your first post.
---
You received this message because you are subscribed to the Google Groups "ClojureScript" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojurescrip...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/7eedfcb0-acff-496c-b332-11589f7e9582n%40googlegroups.com.

[Kovas Palunas]

Ohhhh, I was under the impression that the function was a key function returning a value that would be sorted on place of the real value. sorted-map-by(-key) is how I read it.

Any chance we could make a bug to add this exception?  It would have saved me a lot of time.

You received this message because you are subscribed to a topic in the Google Groups "ClojureScript" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojurescript/0-SJ0zmVX6c/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojurescrip...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/CABgZr0s7iwQe8r%3DWXSDpx42bh7B_T8CURo61U7gonokrw9OJQw%40mail.gmail.com.

[Lauri Pesonen]

Kovas,

I'm just an end-user like yourself so I can't say anything about adding a bug for this. But I don't really see what the language / core library could do differently: the exception already says "wrong number of args (2) passed".

Cheers,

Lauri 

To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/CAPzck%3DdBhpRrkTT65myd4vh8_X4jkoKXWeoXMt%3DKq4hMdPg0oQ%40mail.gmail.com.

[Kovas Palunas]

That's not true in cljs; the maps are just output unsorted without errors.

To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/CABgZr0v1dRNfOKnndPSfGGM%2Bs6kcz509_HO%2B8eiU9yqfYbCL4A%40mail.gmail.com.

[Lauri Pesonen]

Yes, you're right. I suspect it's because JS isn't strict about arity, so it would be difficult / affect perf to do arity checks in CLJS.

Cheers,

Lauri 

To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/CAPzck%3DcE7nd3KFq7zr3L3Trjud0Cyts%2BgK4M_cVW-YRTRhqMaA%40mail.gmail.com.

[Kovas Palunas]

Hmm that makes sense.  I'm still surprised that this works without an error though; to me that implies that you can feed a javascript function with one parameter two parameters instead, and it doesn't complain at all.  Otherwise there must be something weird going on in the CLJS logic?  If this is true than IMO it is a bug.

To view this discussion on the web visit https://groups.google.com/d/msgid/clojurescript/CABgZr0udiw4g8pkVkvQEVcDFhwJtt7yG5hQaA6486d2SFZDJ%3DQ%40mail.gmail.com.

[Henrik Lundahl]

>  to me that implies that you can feed a javascript function with one parameter two parameters instead, and it doesn't complain at all.

Yup:

$ node
Welcome to Node.js v16.14.0.
Type ".help" for more information.
> function identity(x) { return x; }
undefined
> identity(123);
123
> identity(123, "abc");
123
> identity();
undefined
>

--

Henrik