From b60ed4809f1f5cdebdb6fb113fdffdf952383967 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Thu, 17 Oct 2024 15:54:04 +0500 Subject: [PATCH] #1386 fix nextInt and nextLong with extreme boundaries e.g. * `randomService.nextInt(Integer.MIN_VALUE, Integer.MAX_VALUE)` * `randomService.nextLong(Long.MIN_VALUE, Long.MAX_VALUE)` P.S. document the legacy behaviour of `nextLong` method (when min == max) --- .../idnumbers/SingaporeIdNumber.java | 4 +- .../datafaker/providers/base/DateAndTime.java | 4 +- .../datafaker/providers/base/TimeAndDate.java | 4 +- .../net/datafaker/service/RandomService.java | 37 ++++++++++++------ .../providers/base/TimeAndDateTest.java | 15 ++++--- .../datafaker/service/RandomNumbersTest.java | 39 +++++++++++++++++++ .../datafaker/service/RandomServiceTest.java | 31 ++++++++++++--- 7 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 src/test/java/net/datafaker/service/RandomNumbersTest.java diff --git a/src/main/java/net/datafaker/idnumbers/SingaporeIdNumber.java b/src/main/java/net/datafaker/idnumbers/SingaporeIdNumber.java index cf3671456..15c7f09f7 100644 --- a/src/main/java/net/datafaker/idnumbers/SingaporeIdNumber.java +++ b/src/main/java/net/datafaker/idnumbers/SingaporeIdNumber.java @@ -83,9 +83,9 @@ static LocalDate randomBirthDate(BaseProviders faker, Type type) { int now = LocalDate.now().getYear(); return switch (type) { case SINGAPOREAN_TWENTIETH_CENTURY, - FOREIGNER_TWENTIETH_CENTURY -> faker.timeAndDate().birthday(now - 1900, now - 1999); + FOREIGNER_TWENTIETH_CENTURY -> faker.timeAndDate().birthday(now - 1999, now - 1900); case SINGAPOREAN_TWENTY_FIRST_CENTURY, - FOREIGNER_TWENTY_FIRST_CENTURY -> faker.timeAndDate().birthday(now - 2000, now - 2099); + FOREIGNER_TWENTY_FIRST_CENTURY -> faker.timeAndDate().birthday(Math.max(0, now - 2099), now - 2000); }; } diff --git a/src/main/java/net/datafaker/providers/base/DateAndTime.java b/src/main/java/net/datafaker/providers/base/DateAndTime.java index c678f5ea9..e2e961232 100644 --- a/src/main/java/net/datafaker/providers/base/DateAndTime.java +++ b/src/main/java/net/datafaker/providers/base/DateAndTime.java @@ -322,8 +322,8 @@ public Duration duration(long max, ChronoUnit unit) { /** * Generates a random Duration between min and max. * - * @param min the maximum value - * @param max the minimal value + * @param min the minimum value + * @param max the maximum value * @param unit the temporal unit (day or shorter than a day) * @return a random Duration between {@code min} inclusive and {@code max} exclusive if {@code max} greater {@code min}. * @throws IllegalArgumentException if the {@code unit} is invalid. diff --git a/src/main/java/net/datafaker/providers/base/TimeAndDate.java b/src/main/java/net/datafaker/providers/base/TimeAndDate.java index 10b5433cf..30f2f5a7e 100644 --- a/src/main/java/net/datafaker/providers/base/TimeAndDate.java +++ b/src/main/java/net/datafaker/providers/base/TimeAndDate.java @@ -301,8 +301,8 @@ public Duration duration(long max, ChronoUnit unit) { /** * Generates a random Duration between min and max. * - * @param min the maximum value - * @param max the minimal value + * @param min the minimum value + * @param max the maximum value * @param unit the temporal unit (day or shorter than a day) * @return a random Duration between {@code min} inclusive and {@code max} exclusive if {@code max} greater {@code min}. * @throws IllegalArgumentException if the {@code unit} is invalid. diff --git a/src/main/java/net/datafaker/service/RandomService.java b/src/main/java/net/datafaker/service/RandomService.java index 8025e7bb2..e85685e07 100644 --- a/src/main/java/net/datafaker/service/RandomService.java +++ b/src/main/java/net/datafaker/service/RandomService.java @@ -28,12 +28,17 @@ public int nextInt() { return random.nextInt(); } - public int nextInt(int n) { - return random.nextInt(n); + public int nextInt(int maxExclusive) { + return random.nextInt(maxExclusive); } - public Integer nextInt(int min, int max) { - return random.nextInt(min, max + 1); + public Integer nextInt(int minInclusive, int maxInclusive) { + if (minInclusive > maxInclusive) + throw new IllegalArgumentException("Min (%s) > Max (%s)".formatted(minInclusive, maxInclusive)); + if (maxInclusive + 1 < 0) + return (int) nextLong(minInclusive, maxInclusive); + + return random.nextInt(minInclusive, maxInclusive + 1); } @SuppressWarnings("unused") @@ -45,23 +50,33 @@ public long nextLong() { return random.nextLong(); } - // lifted from http://stackoverflow.com/questions/2546078/java-random-long-number-in-0-x-n-range - public long nextLong(long n) { - if (n <= 0) { - throw new IllegalArgumentException("bound must be positive: " + n); + public long nextLong(long maxInclusive) { + if (maxInclusive <= 0) { + throw new IllegalArgumentException("bound must be positive: " + maxInclusive); } long bits, val; do { long randomLong = random.nextLong(); bits = (randomLong << 1) >>> 1; - val = bits % n; - } while (bits - val + (n - 1) < 0L); + val = bits % maxInclusive; + } while (bits - val + (maxInclusive - 1) < 0L); return val; } + /** + * A random long value within given range. + * If {@code min == max} then method always returns {@code min}. + * Otherwise, {@code max} is exclusive. + * + * @param min lower bound (inclusive) + * @param max upper bound (exclusive in most case) + * @return a random long value between {@code min} and {@code max} + */ public long nextLong(long min, long max) { - return min + (long) (nextDouble() * (max - min)); + return min == max ? + min : + random.nextLong(min, max); } public double nextDouble() { diff --git a/src/test/java/net/datafaker/providers/base/TimeAndDateTest.java b/src/test/java/net/datafaker/providers/base/TimeAndDateTest.java index 568872623..41aa6b669 100644 --- a/src/test/java/net/datafaker/providers/base/TimeAndDateTest.java +++ b/src/test/java/net/datafaker/providers/base/TimeAndDateTest.java @@ -173,19 +173,23 @@ void durationTest(long minValue, long maxValue, ChronoUnit unit) { .as("Duration must be equal or greater than min value") .isLessThanOrEqualTo(generated); assertThat(max.compareTo(generated) > 0 || minValue >= maxValue && max.equals(generated)) - .as("Duration must be lower than max value").isTrue(); + .as(() -> "Duration (%s) must be lower than max value (%s) / min value (%s)".formatted(generated, max, min)).isTrue(); } @ParameterizedTest @MethodSource("generateDurationsWithMaxOnly") void durationTest(long maxValue, ChronoUnit unit) { Duration generated = timeAndDate.duration(maxValue, unit); - Duration max = Duration.of(maxValue, unit); - assertThat(max.compareTo(generated) > 0 || maxValue == 0) - .as("Duration must be lower than max value") - .isTrue(); + assertThat(generated) + .as(() -> "Duration (%s) must be lower than max value (%s %s)".formatted(generated, maxValue, unit)) + .isLessThan(Duration.of(maxValue, unit)); } + @Test + void durationIsZero_ifMaxIsZero() { + assertThat(timeAndDate.duration(0, ChronoUnit.DAYS)) + .isEqualTo(Duration.of(0, ChronoUnit.DAYS)); + } @ParameterizedTest @MethodSource("generatePeriod") @@ -196,7 +200,6 @@ void maxLessThanMinPeriod(Period min, Period max) { private static Stream generateDurationsWithMaxOnly() { return Stream.of( - Arguments.of(0, ChronoUnit.DAYS), Arguments.of(100, ChronoUnit.DAYS), Arguments.of(456, ChronoUnit.HOURS), Arguments.of(43, ChronoUnit.MINUTES), diff --git a/src/test/java/net/datafaker/service/RandomNumbersTest.java b/src/test/java/net/datafaker/service/RandomNumbersTest.java new file mode 100644 index 000000000..3d832a93b --- /dev/null +++ b/src/test/java/net/datafaker/service/RandomNumbersTest.java @@ -0,0 +1,39 @@ +package net.datafaker.service; + +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.Test; + +import java.util.SortedSet; +import java.util.TreeSet; +import java.util.function.Supplier; + +import static org.assertj.core.api.Assertions.*; + +class RandomNumbersTest { + private final RandomService randomService = new RandomService(); + + @Test + void nextInt_minInclusive_maxExclusive() { + assertThat(all(() -> randomService.nextLong(3))).containsExactly(0L, 1L, 2L); + assertThat(all(() -> randomService.nextLong(2, 4))).containsExactly(2L, 3L); + assertThat(all(() -> randomService.nextInt(3))).containsExactly(0, 1, 2); + + // inclusive: see https://github.com/datafaker-net/datafaker/issues/1395 + assertThat(all(() -> randomService.nextInt(2, 4))).containsExactly(2, 3, 4); + } + + @RepeatedTest(100) + void nextDouble() { + assertThat(randomService.nextDouble(2, 3)).isGreaterThanOrEqualTo(2); + assertThat(randomService.nextDouble(2, 3)).isStrictlyBetween(1.9999, 3.0); + } + + private > SortedSet all(Supplier lambda) { + SortedSet result = new TreeSet<>(); + for (int i = 0; i < 1000; i++) { + result.add(lambda.get()); + } + return result; + } + +} diff --git a/src/test/java/net/datafaker/service/RandomServiceTest.java b/src/test/java/net/datafaker/service/RandomServiceTest.java index 11b5994ec..1362a3477 100644 --- a/src/test/java/net/datafaker/service/RandomServiceTest.java +++ b/src/test/java/net/datafaker/service/RandomServiceTest.java @@ -2,6 +2,7 @@ import net.datafaker.AbstractFakerTest; import org.assertj.core.api.Condition; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -86,12 +87,26 @@ void predictableRandomRange() { assertThat(f1).isEqualTo(0.41291267F); assertThat(l1).isEqualTo(1092083446069765248L); - assertThat(l2).isOne(); - assertThat(l3).isEqualTo(836L); + assertThat(l2).isEqualTo(1L); + assertThat(l3).isEqualTo(538L); assertThat(b).isFalse(); } + @Test + void equalMinAndMax() { + RandomService randomService = new RandomService(); + assertThat(randomService.nextInt(42, 42)).isEqualTo(42); + } + + @RepeatedTest(10) + void extremeIntegerValues() { + RandomService randomService = new RandomService(); + assertThat(randomService.nextInt(1, Integer.MAX_VALUE)).isBetween(1, Integer.MAX_VALUE); + assertThat(randomService.nextInt(Integer.MIN_VALUE, 0)).isBetween(Integer.MIN_VALUE, 0); + assertThat(randomService.nextInt(Integer.MIN_VALUE, Integer.MAX_VALUE)).isBetween(Integer.MIN_VALUE, Integer.MAX_VALUE); + } + @ParameterizedTest @MethodSource("randomServiceProvider") void testDoubleInRange(RandomService randomService) { @@ -105,13 +120,19 @@ void testDoubleInRange(RandomService randomService) { @ParameterizedTest @MethodSource("randomServiceProvider") void testLongInRange(RandomService randomService) { - final Condition lessThanOrEqual = new Condition<>(t -> t <= 5_000_000_000L, "should be less than or equal 5_000_000_000L"); - final Condition greaterThanOrEqual = new Condition<>(t -> t >= -5_000_000_000L, "should be greater than or equal -5_000_000_000L"); for (int i = 1; i < 1_000; i++) { - assertThat(randomService.nextLong(-5_000_000_000L, 5_000_000_000L)).is(allOf(lessThanOrEqual, greaterThanOrEqual)); + assertThat(randomService.nextLong(-5_000_000_000L, 5_000_000_000L)).isBetween(-5_000_000_000L, 5_000_000_000L); } } + @RepeatedTest(10) + void extremeLongValues() { + RandomService randomService = new RandomService(); + assertThat(randomService.nextLong(0, Long.MAX_VALUE - 1)).isBetween(0L, Long.MAX_VALUE - 1); + assertThat(randomService.nextLong(Long.MIN_VALUE, 0)).isBetween(Long.MIN_VALUE, 0L); + assertThat(randomService.nextLong(Long.MIN_VALUE, Long.MAX_VALUE)).isBetween(Long.MIN_VALUE, Long.MAX_VALUE); + } + @ParameterizedTest @MethodSource("randomServiceProvider") void testHex(RandomService randomService) {