code-reviews 2020-10-20 | Slack Archive

mathpunk15:10:46

Yesterday I learned that conforming returns a pair only when the spec is valid. I also learned that tap> doesn't work in a defmulti for some reason. I'm soliciting comments on the right way of using spec when I have a few alternatives for a piece of data and I want a slightly different function for each one.

(spec/def ::required-data (spec/or :object map?
                                     :singleton (spec/and seq?
                                                          #(= 1 (count %)))))

  (defmulti requirement (fn [{{body :body} :request}]
                          (tap> "it's happening")
                          (tap> (spec/conform ::required-data body))
                          (first (spec/conform ::required-data body)) ))

  (defmethod requirement :object [{{body :body} :request}]
    (select-keys body [:name :parent :id]) )

  (defmethod requirement :singleton [item]
    (requirement (first item)))

mathpunk15:10:22

Don't know how to create ISeq from: clojure.lang.Keyword

mathpunk15:10:34

is what tipped me off that, oh, I guess there are other cases for my data

mathpunk15:10:48

I had thought that conform would throw

seancorfield15:10:41

tap> should work in that context -- the defmulti discriminator is "just code".

seancorfield15:10:07

s/conform will produce ::s/invalid if the data doesn't conform.

seancorfield15:10:32

Hence the exception you get, since you can't take first of ::s/invalid.

seancorfield15:10:16

I'd probably do

(let [data (spec/conform ::required-data body]
  (cond-> data (vector? data) (first)))

adi16:10:07

Alternately, if one wants to explicitly handle the invalid case, one could consider:

(defmulti requirement
  (fn [{{body :body} :request}]
    (let [conformed-value (s/conform ::required-data body)]
      (if-not (s/invalid? conformed-value)
        (first conformed-value)
        ::error))))

(defmethod requirement :object [{{body :body} :request}]
  (select-keys body [:name :parent :id]) )

(defmethod requirement :singleton [item]
  (requirement (first item)))

(defmethod requirement ::error [item]
  ;; return bad request and/or log something
  :boo)

Also it looks like the :singleton method is recursively calling requirement, and it seems to expect a seq. However the dispatch function expects a request map. This confuses me about the shape of data expected.

(requirement {:request {:body {}}}) ; dispatches as :object

(requirement {:request {:body nil}}) ; dispatches as ::error

(requirement :???) ; what will dispatch as singleton?

I believe the current implementation will always dispatch to invalid case for anything except a map shaped like {:request {:body {}}}.

👌 3

noisesmith16:10:13

@U0E9KE222 likely the reason it looks like tap> didn't work is that defmulti has defonce semantics, you need to explicitly destroy a defmulti in order to change it

noisesmith16:10:35

just running defmulti a second time is a no-op if it already exists

mathpunk17:10:17

I definitely just put it in and reevaluated

mathpunk17:10:23

adityaathalye: I believe that the data in question is either a map with those keys, or it is a sequence containing a map with those keys, which is why I think the recursion will work once the discriminator (i needed that word, thanks Sean) is fixed up

mathpunk17:10:40

but! part of the job of this code is to warn me if that belief is incorrect

mathpunk17:10:59

so I will write something that handles the invalid case

mathpunk17:10:26

sean: my first cond->! I was just about to ask you the rationale for using it over an if-statement, but it became clear once I actually started writing it out 🙂

mathpunk17:10:56

thanks folks!

2020-10-20

Channels