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

Method Validator Ignores Null Values / Empty Strings #13

Open
homestar9 opened this issue Jun 13, 2023 · 5 comments
Open

Method Validator Ignores Null Values / Empty Strings #13

homestar9 opened this issue Jun 13, 2023 · 5 comments

Comments

@homestar9
Copy link
Contributor

homestar9 commented Jun 13, 2023

While I was updating the documentation, I noticed the method validator ignores empty/null values. This puzzled me because the udf validator does not ignore empty/null values. I always thought the udf and method validators were similar and could be used interchangeably if you required a more complex function that you wanted to break out from the constraints struct.

Should we allow null/empty strings to pass through to the method and let it decide what to do about null values? In 95% of my udf validators I check for a null/empty value and return true, so maybe the method validator is the one doing it correctly? I see arguments for both approaches so I'm not sure what the best way to proceed is, so I would love some feedback/suggestions.

Method Validator:
https://github.com/coldbox-modules/cbvalidation/blob/0a8dbe8355cfbd4e20db90d0c3be9b2f592b6a2f/models/validators/MethodValidator.cfc#L36-L44

@lmajano
Copy link
Contributor

lmajano commented Jun 13, 2023

I think it makes sense. Things get blurry for me amigo once you have so many validators

@homestar9
Copy link
Contributor Author

homestar9 commented Jun 13, 2023

Thanks @lmajano, so you recommend we leave them as is? Thus the method validator ignores nulls/empty, but the udf validator does not? Or do you propose we change them to be consistent?

@lmajano
Copy link
Contributor

lmajano commented Jun 14, 2023 via email

@homestar9
Copy link
Contributor Author

Sounds good. I will run a few experiments and then create a PR. Stay tuned. :)

@homestar9
Copy link
Contributor Author

homestar9 commented Sep 8, 2023

Created a PR to handle null/empty values that developers can switch to if the UDF validator will treat nulls as valid.
coldbox-modules/cbvalidation#80

Developers should switch to using requiredIf. I added some additional functionality for passing a UDF/closure to that validator as well here: coldbox-modules/cbvalidation#79

I still feel uneasy about implementing the change to the UDF validator, however I can't deny that it does make it more consistent with other validators.

homestar9 added a commit to homestar9/cbvalidation that referenced this issue Sep 8, 2023
This is a breaking change designed to make the UDF validator consistent with other validators as discussed here (ortus-docs/cbvalidation-docs#13).

Developers should switch to the `requiredIf` validator if they need to dynamically evaluate whether a field should be required or not.
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