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

Predicate validation #23

Merged
merged 3 commits into from Feb 18, 2014

Conversation

Projects
None yet
3 participants
@scttnlsn
Copy link
Contributor

scttnlsn commented Feb 18, 2014

Curious how you feel about introducing a validator that accepts arbitrary predicate functions. I find that I already have useful functions that return true or false but validateur expects something of the form [valid? {field #{msg}}].

One potential solution:

(defn validate [attribute predicate & {:keys [message] :or {message "is invalid"}}]
  (fn [m]
    (if (predicate m)
      [true {}]
      [false {attribute #{message}}])))

Example usage:

(validate :email unique-email? :message "is already taken")

Similarly it would be nice to be able to apply validators selectively:

(presence-of :password :when new-user?)

As it is currently, I believe the :when predicate would have to be added separately to each validator. Perhaps some of the :message/:message-fn (and perhaps :when) handling could be extracted into something more reusable.

Alternatively, I'm currently using:

(defn validate-when [predicate validator]
  (fn [m]
    (if (predicate m)
      (validator m)
      [true {}])))

Example usage:

(validate-when new-user? (presence-of :password))

I'd be happy to submit a pull request if any of this is something you're interested in including.

@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 17, 2014

validate-when looks good but I'm not sure I understand how it is used in context. Modifying existing validators and making predicates work with functions that produce error messages can get messy.
validate-when sounds interesting.

@scttnlsn

This comment has been minimized.

Copy link
Contributor

scttnlsn commented Feb 17, 2014

Yeah, forget about:

(presence-of :field :when predicate?)

I agree that it could get messy and it's unnecessary. I find validate-when plenty useful for any kind of conditional validation. Here's a more complete example of validating a user before saving to a database:

(defn new? [user]
  (nil? (:id user)))

(defn unique-email? [user]
  (if-let [existing (find-by-email (:email user)]
    (= (:id user) (:id existing))
    true))

(def validate
  (validation-set
    (presence-of :email)
    (validate :email unique-email? :message "is already taken")
    (validate-when new? (presence-of :password))
    (validate-when #(contains? % :password) (presence-of :password))))

Does that make more sense?

@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 17, 2014

Yes, it does now. Please submit a pull request. Thank you!

@scttnlsn

This comment has been minimized.

Copy link
Contributor

scttnlsn commented Feb 18, 2014

Added the two functions and tests. The generic validate function needs a better name. Any suggestions?

Example:
(use 'validateur.validation)

This comment has been minimized.

@michaelklishin

michaelklishin Feb 18, 2014

Owner

Don't use clojure.core/use.

This comment has been minimized.

@scttnlsn

scttnlsn Feb 18, 2014

Contributor

That's just in the comment and I included it to be consistent with the rest of the examples. Perhaps changing all the examples is best saved for another issue?

This comment has been minimized.

@michaelklishin
@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 18, 2014

I'd rename validate to validate-with-predicate but that's not great either. However, it is to the point
and will not be confused with valid?.

@hura

This comment has been minimized.

Copy link
Contributor

hura commented Feb 18, 2014

Why not give acceptance-of a parameter accept-fn instead of introducing validate-with-predicate. Just a thought. Feel free to ignore.

michaelklishin added a commit that referenced this pull request Feb 18, 2014

@michaelklishin michaelklishin merged commit 58edf29 into michaelklishin:master Feb 18, 2014

1 check passed

default The Travis CI build passed
Details
@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 18, 2014

In 2.0.0-beta1.

@scttnlsn

This comment has been minimized.

Copy link
Contributor

scttnlsn commented Feb 18, 2014

Great- thanks!

@scttnlsn

This comment has been minimized.

Copy link
Contributor

scttnlsn commented Feb 18, 2014

@michaelklishin Just tried using 2.0.0-beta1 but I get java.lang.RuntimeException: No such var: v/validate-with-predicate. Looking inside the JAR I noticed that, indeed, the two functions I added are missing.

@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 18, 2014

I guess the cljx stuff needs manual cross-compilation every time. Sigh.

@michaelklishin

This comment has been minimized.

Copy link
Owner

michaelklishin commented Feb 18, 2014

Try 2.0.0-beta2.

@scttnlsn

This comment has been minimized.

Copy link
Contributor

scttnlsn commented Feb 18, 2014

It's working. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment