Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we have a phrase-all function? #15

Open
ouvanous opened this issue Apr 24, 2018 · 6 comments
Open

Should we have a phrase-all function? #15

ouvanous opened this issue Apr 24, 2018 · 6 comments
Labels

Comments

@ouvanous
Copy link

Hi,
Not sure if i understood all correctly but i don't think that there is a method to get all the errors ?
like :

(defn phrase-all
  [context spec x]
  (some->> (s/explain-data spec x)
           ::s/problems
           (map #(phrase context %))))

Do i miss something ?
Thanks,

@alexanderkiel
Copy link
Owner

You are right. There is no phrase-all function. In the README I explain why:

However, you have to use phrase directly if you like to phrase more than one problem. The library doesn't contain a phrase-all function because it doesn't know how to concatenate messages.

Returning a list of messages from phrase-all might be a viable solution. So I let the issue open for votes to provide a phrase-all as you suggested.

@alexanderkiel alexanderkiel changed the title phrase-all Should we have a phrase-all function? Apr 24, 2018
@leblowl
Copy link
Contributor

leblowl commented Jul 2, 2018

I think it's a nice idea, some folks like to return all errors at once for a particular API input - phrase-all would make that easy

@alexanderkiel
Copy link
Owner

Do you also think, that a simple list of messages is a good solution?

@leblowl
Copy link
Contributor

leblowl commented Jul 2, 2018

@alexanderkiel yes that's what I would expect

@jsa-aerial
Copy link

Here's what I have for this - it works very well:

(defn validate-msg
  [spec x & {:keys [sep prefix suffix with-val?]
             :or {sep " and " prefix "" suffix "" with-val? false}}]
  (let [msgbody (->> (s/explain-data spec  x) ::s/problems
                     (mapv #(phrase {} %))
                     (cljstr/join sep))
        msgval (if with-val? (format "<%s>: " x) "")]
    (str msgval prefix msgbody suffix)))

(defn make-validator [spec & validate-msg-kvs]
  (fn [x]
    (if (not (s/valid? spec x))
      (apply validate-msg spec x validate-msg-kvs)
      "")))

;;; Example use:
(def validate-bc-rep-xref (make-validator ::bc-replicate-xref-rec))

(validate-bc-rep-xref
 {:strain-cond-repid (str/split #"-" "T4P-006PEN120MIN-CC"),
  :illumbc "TACTTAG",
  :sampbc "TGGTCCTx"})
=> "rep id `CC` in `T4P-006PEN120MIN-CC` must be a single upper case or lower case letter or digit and Sample BC `TGGTCCTx` must contain only 'A', 'T', 'G', 'C' characters and Illumina BC `TACTTAG` is not listed as an index in SampleSheet.csv"

@reutsharabani
Copy link

I think this is required to provide messages to consumers. Its very unpleasant to provide one error at a time.

This suggests users of this library will create their phrase-all for any non-trivial use-case (probably most use cases).

Therefore I support adding it to the core of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants