Copilot commented on code in PR #1817:
URL: https://github.com/apache/commons-lang/pull/1817#discussion_r4272431306
##########
src/test/java/org/apache/commons/lang3/RandomUtilsTest.java:
##########
@@ -245,6 +245,19 @@ void testNextDouble(final RandomUtils ru) {
assertTrue(result < 42d);
}
+ /**
+ * Test a double range one ULP wide. A previous implementation could round
the scaled value up to the
+ * exclusive upper limit about half of the time.
+ */
+ @Test
+ void testNextDoubleExclusiveUpperBound() {
+ final double startInclusive = 10d;
+ final double endExclusive = Math.nextUp(startInclusive);
+ for (int i = 0; i < 1000; i++) {
+ assertEquals(startInclusive,
RandomUtils.nextDouble(startInclusive, endExclusive));
+ }
+ }
Review Comment:
The new exclusive-upper-bound tests duplicate the same structure (start
value, `nextUp`, and `for (int i = 0; i < 1000; i++)`) across 4 methods.
Consider extracting a small private helper (and/or a shared `TRIALS` constant)
to reduce duplication and make future adjustments (e.g., changing the trial
count or start value) less error-prone.
##########
src/main/java/org/apache/commons/lang3/RandomUtils.java:
##########
@@ -345,7 +345,9 @@ public double randomDouble(final double startInclusive,
final double endExclusiv
if (startInclusive == endExclusive) {
return startInclusive;
}
- return startInclusive + (endExclusive - startInclusive) *
random().nextDouble();
+ final double result = startInclusive + (endExclusive - startInclusive)
* random().nextDouble();
+ // Rounding in the scaling above can reach endExclusive; clamp to the
greatest value below endExclusive.
+ return result < endExclusive ? result : Math.nextDown(endExclusive);
Review Comment:
This clamping logic is correct for the instance methods, but it’s easy for
similar scaling code paths to diverge (e.g., any static
`nextDouble`/`nextFloat` implementation or other internal helpers that compute
`start + (end-start) * u`). To keep behavior consistent and avoid regressions,
consider centralizing the scaling+clamp into a shared private helper used by
all floating-point range methods.
--
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]
