Skip to content

Commit 30342e9

Browse files
committed
Remove unused logFactorial method
1 parent d6ee9ba commit 30342e9

2 files changed

Lines changed: 6 additions & 31 deletions

File tree

commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/InternalUtils.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@
2727
*/
2828
final class InternalUtils {
2929
/** All long-representable factorials, precomputed as the natural
30-
* logarithm using Matlab R2023a VPA: log(vpa(x)).
31-
*
32-
* <p>Note: This table could be any length. Previously this stored
33-
* the long value of n!, not log(n!). Using the previous length
34-
* maintains behaviour. */
30+
* logarithm using Matlab R2023a VPA: log(vpa(x)). */
3531
private static final double[] LOG_FACTORIALS = {
3632
0,
3733
0,
@@ -68,16 +64,6 @@ final class InternalUtils {
6864
/** Utility class. */
6965
private InternalUtils() {}
7066

71-
/**
72-
* @param n Argument.
73-
* @return {@code n!}
74-
* @throws IndexOutOfBoundsException if the result is too large to be represented
75-
* by a {@code long} (i.e. if {@code n > 20}), or {@code n} is negative.
76-
*/
77-
static double logFactorial(int n) {
78-
return LOG_FACTORIALS[n];
79-
}
80-
8167
/**
8268
* Validate the probabilities sum to a finite positive number.
8369
*

commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/InternalUtilsTest.java

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,34 +31,23 @@ class InternalUtilsTest {
3131
private static final int MAX_REPRESENTABLE = 20;
3232

3333
@Test
34-
void testFactorial() {
35-
Assertions.assertEquals(0L, InternalUtils.logFactorial(0));
34+
void testFactorialLogNoCache() {
35+
FactorialLog factorialLog = FactorialLog.create();
36+
Assertions.assertEquals(0, factorialLog.value(0));
3637
long result = 1;
3738
for (int n = 1; n <= MAX_REPRESENTABLE; n++) {
3839
result *= n;
3940
final double expected = Math.log(result);
40-
Assertions.assertEquals(expected, InternalUtils.logFactorial(n), Math.ulp(expected));
41+
Assertions.assertEquals(expected, factorialLog.value(n), Math.ulp(expected));
4142
}
4243
}
4344

44-
@Test
45-
void testFactorialThrowsWhenNegative() {
46-
Assertions.assertThrows(IndexOutOfBoundsException.class,
47-
() -> InternalUtils.logFactorial(-1));
48-
}
49-
50-
@Test
51-
void testFactorialThrowsWhenNotRepresentableAsLong() {
52-
Assertions.assertThrows(IndexOutOfBoundsException.class,
53-
() -> InternalUtils.logFactorial(MAX_REPRESENTABLE + 1));
54-
}
55-
5645
@Test
5746
void testFactorialLog() {
5847
// Cache size allows some of the factorials to be cached and some
5948
// to be under the precomputed factorials.
6049
FactorialLog factorialLog = FactorialLog.create().withCache(MAX_REPRESENTABLE / 2);
61-
Assertions.assertEquals(0, factorialLog.value(0), 1e-10);
50+
Assertions.assertEquals(0, factorialLog.value(0));
6251
for (int n = 1; n <= MAX_REPRESENTABLE + 5; n++) {
6352
// Use Commons math to compute logGamma(1 + n);
6453
final double expected = Gamma.logGamma(1 + n);

0 commit comments

Comments
 (0)