Suggestions for some JUnit test improvements in Refactoring the Traditional “for” loop p 181 ff

Below code for testing the original/classical “factorial” and the stream-based “factorial” .

The following changes have been made:

• The Factorial class has been given a constructor printing something when it is called (just for fun)
• Creating BigInteger instances is annoying, so we use the bigOf() static method to create them.
• The computeFactorial() function throws an exception if negative values are passsed in, as it should.
• In order to compare an existing class’ behaviour against the behaviour of newly implemented class (supposed to be a drop-in replacement), we need to identify the methods relevant to the test, factor them out into a new interface, and have the new and old classes implement that interface (in the real world, fixing the old code in that way may not always be possible though). Proceeding according to this recipe allows us to use the same test code to exercise the old and new classes with the same test harness - no test code duplication is needed for the two separate classes.
• There is no valid reason to have a @BeforeEach method as shown in the book, as that method is only called once even in the presence of assertAll(). It is cleaner to move the initialization of Factorialinto the individual @Test methods to keep everything local.
• Testing of some “failure cases” (here, one case) has been added. Those are often forgotten in real life.

package chapter11;

import org.junit.jupiter.api.Test;

import java.math.BigInteger;
import java.util.stream.LongStream;

import static org.junit.jupiter.api.Assertions.*;

public class TraditionalForLoopTest {

    interface Factorial {

        BigInteger compute(long upTo);

    }

    static class FactorialBefore implements Factorial {

        public FactorialBefore() {
            System.out.println("Created " + getClass().getName());
        }

        public BigInteger compute(long upTo) {
            if (upTo < 0) {
                throw new IllegalArgumentException("The passed values is < 0: " + upTo);
            }
            BigInteger result = BigInteger.ONE;
            for (int i = 1; i <= upTo; i++) {
                result = result.multiply(BigInteger.valueOf(i));
            }
            return result;
        }

    }

    static class FactorialAfter implements Factorial {

        public FactorialAfter() {
            System.out.println("Created " + getClass().getName());
        }

        public BigInteger compute(long upTo) {
            if (upTo < 0) {
                throw new IllegalArgumentException("The passed values is < 0: " + upTo);
            }
            return LongStream.rangeClosed(1, upTo)
                    .mapToObj(BigInteger::valueOf)
                    .reduce(BigInteger.ONE, BigInteger::multiply);
        }

    }

    private static BigInteger bigOf(long x) {
        return BigInteger.valueOf(x);
    }

    private static void commonFactorialTests(final Factorial factorial) {
        assertAll(
                () -> assertEquals(bigOf(1), factorial.compute(0)),
                () -> assertEquals(bigOf(1), factorial.compute(1)),
                () -> assertEquals(bigOf(2), factorial.compute(2)),
                () -> assertEquals(bigOf(6), factorial.compute(3)),
                () -> assertEquals(bigOf(120), factorial.compute(5)),
                () -> assertThrows(IllegalArgumentException.class, () -> factorial.compute(-1))
        );
    }

    @Test
    void factorialBefore() {
        commonFactorialTests(new FactorialBefore());
    }

    @Test
    void factorialAfter() {
        commonFactorialTests(new FactorialAfter());
    }

}