[Validator] Improve DivisibleBy validator #28212
Conversation
|
Not sure whether this is a bug fix or a new feature, though. I would say it is a bug fix (or an improvement), but I have not hard stance there. |
| $epsilon = 0.0000001; | ||
|
|
||
| // can't divide by 0 | ||
| if ($value2 < $epsilon) { |
There was a problem hiding this comment.
Shouldn't this trigger an exception when the constraint is configured with such an invalid value?
There was a problem hiding this comment.
Not sure. The previous implementation with fmod just returned NaN (so the validator failed).
We can change it, I just adapted the previous behavior.
| return (float) 0 === fmod($value1, $value2); | ||
| $value1 = \abs($value1); | ||
| $value2 = \abs($value2); | ||
| $epsilon = 0.0000001; |
There was a problem hiding this comment.
What about making this configurable?
There was a problem hiding this comment.
I‘m not sure whether this would actually add anything.
Might make sense to add something like FloatUtils to capsiluate some of this logic (comparing floats etc)
|
I proposed an alternative in #28228 which should handle more precision. |
…ibleBy constraint (apfelbox) This PR was merged into the 4.2-dev branch. Discussion ---------- [Validator] Fix precision issue regarding floats and DivisibleBy constraint | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Alternative to #28212 Commits ------- ae04b48 [Validator] Fix precision issue regarding floats and DivisibleBy constraint
The newly added
DivisibleByvalidator uses internallyfmod, which has some issues regarding floating point numbers and numbers with decimals.For example
fmod(4.1, 0.1) ~= 0.1and currently fails the validator. See for example this comment in the PHP docs: http://de2.php.net/manual/en/function.fmod.php#122782This change implements the logic manually ( 😞 ) but should solve more edge cases.