2023-01-13 code-reviews | Clojure Slack Archive

code-reviews

mathpunk 2023-01-13T20:40:47.354389Z

Here is my dumb little function for paring down a given data shape:

(defn failures [{:keys [results]}]
  (if-not (= 1 (count results))
    (throw "Expectations failed: There was more than one result")
    (let [suites (-> (first results) (get-in [:suites]))]
      (if-not (= 1 (count suites))
        (throw "Expectations failed: There was more than one suite")
        (->> (-> suites first (get :tests))
             (filter :fail))))))

By inspection, I’ve observed that it looks like the results and suites are always a sequence containing one map, rather than a map. I dunno why, it’s just what gets spit out. But what if I’m wrong? I’ll want to know, and update my assumptions. I feel like this if-not… throw pattern is probably better handled by spec, right? When you’re trying to be quick and dirty and use an observation, but you know it might be wrong in the future and you want to do future self a favor, how would you handle this?

jaihindhreddy 2023-01-15T11:33:58.726439Z

I find this funny:

(defn failures
  [{[{[{:keys [tests]}
       :as suites] :suites}
     :as results] :results}]
  {:pre [(= 1 (count results))
         (= 1 (count suites))]}
  (filter :fail tests))

moe 2023-01-15T11:38:15.081989Z

:keys is truly an abomination

moe 2023-01-15T11:39:39.952379Z

but so is that indentation

🤣 1

jaihindhreddy 2023-01-15T11:41:08.108189Z

That's https://github.com/tonsky/Clojure-Sublimed#formatterindenter.

moe 2023-01-15T11:56:32.580939Z

(defn failures {[{[{tests :tests} & suites] :suites} & results] :results}]
  {:pre [(not suites) (not results)]}
  (filter :fail tests))

jaihindhreddy 2023-01-15T11:57:55.529919Z

yeah, just single line looks much better. Still. That's quite a bit going-on in the arglist. Also, the two & should be :as AFAICT.

moe 2023-01-15T11:58:25.340619Z

No, it's taking the tail of the vectors to make the precondition simpler

💯 1

jaihindhreddy 2023-01-15T11:59:35.915409Z

oh yeah, didn't read the changed pre-conditions. Nice!

moe 2023-01-15T12:01:48.853819Z

but yeah, that's not a winning function signature

escherize 2023-02-07T20:27:56.520169Z

moe 2023-01-14T05:43:53.886579Z

If prefer when for throwing exceptions. You could use function pre-conditions for these guys if you didn't want to go full spec

💡 1

moe 2023-01-14T05:47:55.054269Z

{:pre [(= 1 (count results)) (-> results first :suites count (= 1))]}

moe 2023-01-14T05:50:22.817189Z

It's a personal thing, but I never use the keys destructuring form. I think it looks incongruous. Even if I'm mapping keywords to vars in the same way, I'll just spell it out

2023-01-14T23:42:17.421359Z

Containers which I only expect to have one member happens often enough that I keep a special utility function.

(defn unique
  "Returns the single member of `coll`, or nil if `coll` is empty. Calls `on-ambiguity` if there is more than one member (default is to throw an Exception).
  Where
  -   `coll` is a collection
  -   `on-ambiguity` := (fn [coll] ...) -> `value`, default raises an error.
 
  "
  ([coll on-ambiguity]
   (if (seq coll)
     (if (> (count coll) 1)
       (on-ambiguity coll)
       (first coll))))
  ([coll]
   (unique coll (fn [coll]
                  (throw (ex-info "Unique called on non-unique collection"
                                  {:type ::NonUnique
                                   :coll coll
                                   }))))))

Using that, you could re-write your code as something like:

(defn failures [m]
  (let [suite (-> m
                 (:results)
                 (unique)
                 (:suites)
                 (unique)
         ]
   (filter :fail (:tests suite)))

🙏🏻 1

2023-01-14T23:51:36.288779Z

Having looked at this code for the first time in a while, I think the preferred order for those two versions of unique should be in the opposite order with the shorter arg list first. Also, I should probably` (assert (coll seq))` and not make it a conditional.

📝 1

mathpunk 2023-01-15T00:38:19.400519Z

thanks y'all!

❤️ 1

Clojurians Log v2

code-reviews