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

Composable validation for nested sequences? #11

Closed
trentonstrong opened this issue Aug 22, 2013 · 3 comments
Closed

Composable validation for nested sequences? #11

trentonstrong opened this issue Aug 22, 2013 · 3 comments

Comments

@trentonstrong
Copy link

I have been running into cases in my project using bouncer where I have a map to be validated that contains keys which map to sequences of further maps. Your person-with-pets example is a good one:

(def person-with-pets {:name "Leo"
                       :pets [{:name nil}
                              {:name "Gandalf"}]})

(b/validate person-with-pets
          :pets [[v/every #(not (nil? (:name %)))]])

When validating with every you are checking for the presence the name key on the nested maps ad-hoc, but this could very well be done with a separate validation function specific to pets. The issue with that is every simply expects a truth-y value, and will be unaware if the call was to another validation routine that returns its own set of validation errors for a specific map in the sequence.

Of course, one workaround to this would be to validate the pets individually and separately from the person validation, but often it makes sense to keep these validations in one place (maybe!).

Any thoughts on this? I would be willing to work on a PR if this seems like a good addition and got a little nudge in the right direction.

@theleoborges
Copy link
Owner

Hi @trentonstrong ,

Thanks for filing the issue.

The more I think about it, the more I'm convinced adding every wasn't such a good idea.

The reason is that if you're validating several items in a collection at once, the usefulness of displaying every single validation error is greatly diminished.

Say you're displaying this on a web page. Your users would just be overwhelmed with so many validation errors coming through at once. A better approach would be to validate each pet as they are added in the UI and display feedback in small chunks.

It all comes down to what you're using the validation results for.

Another reason I think every should be removed eventually is that it can be simulated with a validation function:

(def pet-validations {:name v/required})

(v/defvalidator pets-validator
  [pets]
  (-> (some false?
            (reduce (fn [acc pet]
                            (conj acc (core/valid? pet pet-validations)))
                        [] pets))
      boolean
      not))

(core/validate person-with-pets
               :pets pets-validator)

However if you're really in the scenario where you need the specific validation errors for each member of the collection, I would go down the route of validating them individually for now.

Personally I haven't found that scenario yet. Since you filed the issues, I'm assuming you have? If that's the case, I'd love to learn more about about the specifics. We can move on from there :)

Also, thanks for offering to work on a pull request. I'll more than happy put you in the right direction should we decide to go down that path :)

@trentonstrong
Copy link
Author

Thanks for the quick reply, @LeonardoBorges!

After reading through your thoughtful response, I tend to agree with you. My specific scenario involves validating a blob of configuration data that is being sent from one system to another in order to perform a job (run a model), wherein a lot of domain objects describing the model need to be included in the configuration are going to be used all together to formulate a model. So there isn't really a workaround (that I have thought of) akin to adding each individual pet in the UI, though the program on the sending end of this system will be doing that kind of granular validation.

I was already deliberating the granularity of errors that would be valuable in reporting back to the sender system if the job was invalid and now I am pretty sure you are correct that the value is in detecting an error rather than all of the specific (which could be quite large in number) violations.

The approach simply detecting whether or not there was an invalid member of the sequence seems a bit more sane, especially as reproducing the error will require some investigation anyways.

@theleoborges
Copy link
Owner

Hi @trentonstrong ,

I'm glad my reply was useful. As it currently stands, I'll close this issue as it seems we're both in agreement that this should be handled outside of bouncer.

Thanks for bringing it to my attention anyway.

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