fix issue 15763 - Document behavior of relative difference #5316
Conversation
issue where symmetry was assumed when lhs was a value and rhs was not.
|
|
Should have a test case |
| // lhs is number, rhs is array | ||
| return approxEqual(rhs, lhs, maxRelDiff, maxAbsDiff); | ||
| // lhs is number, rhs is range | ||
| for (; !rhs.empty; rhs.popFront()) |
There was a problem hiding this comment.
I know it's because you're copying the implementation from the other branch, but it's odd to not use foreach here
There was a problem hiding this comment.
Feel free to change or ignore, not really a pressing issue
There was a problem hiding this comment.
The difference between foreach and this is that foreach creates a copy, this doesn't. I don't know what the implications are in terms of ranges that may fail or have weird behavior because of this. As I'm not focused on that aspect, I'd prefer to leave the implementation as is, and worry about this in a separate pull.
I will note that the original implementation does make a copy when recursing, so in this case it's not a big deal. However, it would look odd to use foreach here and not in the other location.
I will look into adding one. Should be easy enough. |
|
test added @JackStouffer |
|
ping, this is pretty trivial. |
|
Thx @JackStouffer |
Also fix issue where symmetry was assumed when lhs was a value and rhs was not. Instead of swapping the arguments (which clearly isn't equivalent), I copied the implementation of the range processing, but for rhs instead of lhs.