weixin_39607450
weixin_39607450
2020-12-01 14:28

Only existing input in ValidationResult values

Currently the ValidationResult::getValues() will return both values that were part of the input and values that weren't. Optional values will currently get false as filling, or null after my other pull-request. This means that optional values are filled by validation and returned as validated values, while the given validated values are not validated at all and basicly faked. This change aims to leave out optional values.

To go over the current options, and why this change is proposed:

Current: fill optional empty fields with false This is wrong, false may be a perfectly valid value and thus gets a double meaning. Where it being left out (validly as it is optional) returns it as set to false. This ambiguity is very dangerous.

Alternatively after my pull request for Container::traverse() This makes it filled with null. While null would be valid in the sense that it denotes an empty field, it does pose other problems. For example, a database field might be either empty (null) or have a valid value (non-null). If null becomes the default value I can't distinguish if it was left empty because of a partial update or intentionally emptied to become null.

Leave out non-existent fields from validation result This is what this PR proposed. Any fields not part of the input should not be in the ValidationResult. They are either optional and thus validly not part of the output. Or they are required, didn't validate and shouldn't be in the getValues() output.

该提问来源于开源项目:particle-php/Validator

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

5条回答

  • weixin_39831104 weixin_39831104 5月前

    Glancing over, this looks much better! I'll review in detail shortly. Thanks for the input!

    点赞 评论 复制链接分享
  • weixin_39831104 weixin_39831104 5月前

    Looks really good, thanks for the contribution! I'm merging this.

    点赞 评论 复制链接分享
  • weixin_39831104 weixin_39831104 5月前

    Hey , good point! I guess I didn't test this through thoroughly enough. Anyway, musing over the options you've laid out above I would definitely choose the last one. However, I'm not sure if I'm (yet) convinced that this is the place to fix it, because it should be possible to do this while writing to the Value\Container output. I guess this could also probably be fixed by adding the value to the output container conditionally (so only if the value existed).

    That will lead to less loops, and less cyclomatic complexity. Also, I'd very much like to see a unit test to prove that it works as expected now. That way we can be sure that upon writing new features, we don't break this piece of code.

    点赞 评论 复制链接分享
  • weixin_39607450 weixin_39607450 5月前

    Fair point, this isn't the optimal solution - more of a practical one I'm using as a wrapper around the Particle\Validator class.

    I'll write up a unit test (for #101 as well) at least to show how I expect it to work and which will pass with this PR. For a less cyclomatic complex solution I'd need to dive into how optional is handled, I probably won't find the time to do that any time soon.

    点赞 评论 复制链接分享
  • weixin_39607450 weixin_39607450 5月前

    -langerak this is a bit better then the previous solution, and unit-tested.

    点赞 评论 复制链接分享

相关推荐