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

Add assertion functionality for pre/post conditions. #12

Closed
ToBeReplaced opened this issue Sep 11, 2013 · 4 comments
Closed

Add assertion functionality for pre/post conditions. #12

ToBeReplaced opened this issue Sep 11, 2013 · 4 comments

Comments

@ToBeReplaced
Copy link

What would you think about adding functionality like:

(ns example
  (:require [clojure.pprint :refer [pprint]]
            [clojure.string :refer [join]]
            [bouncer.core :refer [validate]]
            [bouncer.validators :refer [required number]]))

(defn assert-valid
  "Throws an AssertionError if m is not valid with a validation error
  as its message."
  [m & forms]
  (when *assert*
    (let [errors (first (apply validate m forms))]
      (when (seq errors)
        (->> errors
             (reduce-kv (fn [acc k error]
                          (assoc acc k {:value (get m k) :error error}))
                        {})
             pprint
             with-out-str
             (vector "Validation failed with the following errors:")
             (join \newline)
             AssertionError.
             throw)))))

(assert-valid {:foo "hi" :bar 1}
  :foo [number]
  :bar [number]
  :baz [required])
;; AssertionError Validation failed with the following errors:
;; {:foo {:value "hi", :error ("foo must be a number")},
;;  :baz {:value nil, :error ("baz must be present")}}
;;   example/assert-valid (example.clj:20)

This would be useful in :pre and :post conditions instead of valid?, since valid? gives no visibility into how/why it returned false.

@ToBeReplaced
Copy link
Author

Obviously that's not quite right... it would need to be a macro to disappear on compilation, but that's the general idea. I'd be happy to work on it if this functionality is desirable.

Also, do we still need to return a list for each key? The short-circuiting means only one error is returned for each key.

@ToBeReplaced
Copy link
Author

And now I think this is probably a bad idea. Assertion methods should return nil (as this does), but then it is not suitable for use in a pre/post conditions. This would just be a utility method to stick in code during debugging, but then the needs are just as easily met by (assert (or (valid? args) (pr-str args)). I'm interested in feedback.

@theleoborges
Copy link
Owner

Sorry about the delay. I'll have a think about this and come back to you.

Regarding returning lists, that's there for backwards compatibility - the short-circuiting mechanism hasn't always been there.

@theleoborges
Copy link
Owner

Hi @ToBeReplaced ,

I think this feature should probably be left out of the core functionality and into the user code.

It seems to be mostly concerned with control flow and error presentation. For that reason, I'm closing this issues.

Feel free to re-open it if you think you have better arguments as to why this should be in the core lib.

Thanks

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

No branches or pull requests

2 participants