aherbert commented on code in PR #396:
URL:
https://github.com/apache/commons-collections/pull/396#discussion_r1228013793
##########
src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java:
##########
@@ -99,7 +99,7 @@ void testModEdgeCases() {
for (final long dividend : new long[] {-1, -2, -3, -6378683,
-23567468136887892L, Long.MIN_VALUE, 345, 678686,
67868768686878924L, Long.MAX_VALUE}) {
for (final int divisor : new int[] {1, 2, 3, 5, 13,
Integer.MAX_VALUE}) {
- assertEquals((int) Long.remainderUnsigned(dividend, divisor),
EnhancedDoubleHasher.mod(dividend, divisor),
+ assertEquals((int) Long.remainderUnsigned(dividend, divisor),
BitMap.mod(dividend, divisor),
Review Comment:
When doing this can you add an extra case for `dividend=0`. This is not
currently covered.
I don't think we need edge cases to show behaviour when the divisor is
negative. This method is written for performance and omits checks for bad
arguments. However the javadoc can be updated to make it a user beware method.
It should add strong wording that the result is undefined if the divisor is
negative, and there will be an java.lang.ArithmeticException if the divisor is
zero.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
