2020-10-20 code-reviews | Clojure Slack Archive

code-reviews 2020-10-20

mathpunk 2020-10-20T15:43:46.043700Z

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

mathpunk 2020-10-20T15:44:22.043800Z

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

mathpunk 2020-10-20T15:44:34.044Z

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

mathpunk 2020-10-20T15:44:48.044200Z

I had thought that conform would throw

seancorfield 2020-10-20T15:53:41.044400Z

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

seancorfield 2020-10-20T15:54:07.044600Z

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

seancorfield 2020-10-20T15:54:32.044800Z

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

seancorfield 2020-10-20T15:56:16.045Z

I'd probably do

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

adi 2020-10-20T16:49:07.045200Z

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 {}}}.

👌 1

2020-10-20T16:55:13.045500Z

@mathpunk 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

2020-10-20T16:55:35.045700Z

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

mathpunk 2020-10-20T17:09:17.046Z

I definitely just put it in and reevaluated

mathpunk 2020-10-20T17:14:23.046200Z

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

mathpunk 2020-10-20T17:14:40.046400Z

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

mathpunk 2020-10-20T17:14:59.046600Z

so I will write something that handles the invalid case

mathpunk 2020-10-20T17:17:26.046800Z

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 🙂

mathpunk 2020-10-20T17:17:56.047Z

thanks folks!

Clojurians Log v2

code-reviews 2020-10-20