brightcove / valerie

First class validation library for the JVM
Other
3 stars 0 forks source link

Don't use `define` with map arg #27

Open mwhipple opened 8 years ago

mwhipple commented 8 years ago

define got abused after being adopted by using it to both define the current value and also the children (was easier than thinking of another name)

has is suggested to replace the latter use of define, which would result in something like:

...user: {
  has firstName: { isInstanceOf String }
       lastName: { isInstanceOf String }
}

the use of "has" could also suggest a friendlier complementary term than "define"...but so far I don't have any ideas that I haven't talked myself out of...though "that" would probably be the front-runner.

hasOnly

in addition to this adding a hasOnly could be added to verify that no fields exist beyond those that are provided in the keySet, so if the above were rewritten to use hasOnly then it would be equivalent to including a hasOnlyFieldsIn(['firstName', 'lastName']). In the (theoretical) builder syntax this would have to be handled either by some kind of pragma within the closure such as:

user {
  v.hasOnlyThese
  firstName ( isInstanceOf String )
  lastName ( isInstanceOf String )
}

or a a special check on the object like:

user ( hasOnlyFieldsListed() ) {
  firstName(...
  ...
}

I'd lean towards the second since it seems more consistent...but either should also allow setting a global default with a solution to selectively choose the other option to accommodate either style of validation.

In the event the hasOnly style is preferred the builder syntax would be convenient to just list fields without validating them such as

user {
  firstName
  lastName
}

while the map syntax would require something as a value...presently the standard pass would work but some additional sugar would probably be desirable.