Extensible and testable FizzBuzz

There seem no end to fizzbuzz implementations.
Something I rarely see is an implementation that's easy to extend with more name-divisor pairs.
Another thing that usually annoys me is the lack of testability.
Printing to standard output and verifying by reading is less than ideal.

This FizzBuzz class can work with an arbitrary number of fizzers,
following these steps:

• Get a builder with FizzBuzz.builder()

• Add the fizzers using the builder, for example for classic fizz-buzz:

• Call .add("Fizz", 3)

• Call .add("Buzz", 5)

• Create a FizzBuzz object by calling .build() when ready

• The .getValue(number) method returns the value for a specified number

The code:

public class FizzBuzz {
    private final List fizzers;

    private static class Fizzer {
        private final String name;
        private final int divisor;

        private Fizzer(String name, int divisor) {
            this.name = name;
            this.divisor = divisor;
        }
    }

    public static class Builder {
        private final List fizzers = new ArrayList<>();

        public Builder add(String name, int divisor) {
            fizzers.add(new Fizzer(name, divisor));
            return this;
        }

        public FizzBuzz build() {
            return new FizzBuzz(this);
        }
    }

    private FizzBuzz(Builder builder) {
        fizzers = Collections.unmodifiableList(builder.fizzers);
    }

    public String getValue(int num) {
        String output = "";
        for (Fizzer fizzer : fizzers) {
            if (num % fizzer.divisor == 0) {
                output += fizzer.name;
            }
        }
        return output.isEmpty() ? Integer.toString(num) : output;
    }

    public static Builder builder() {
        return new Builder();
    }
}

A sample demo program using classic fizz-buzz rules,
and printing results from 1 to 100:

```
import static java.util.stream.IntStream.rangeClosed;

class FizzBuzzDemo {
public static void main(String[

You implementation makes assumptions about how Builder.add() is called. If they are not explicitly documented, they are errors in the code.

-
Divisors not added in increasing order:

@Test
public void BuilderAdd_DifferentDivosorOrder_SameOutput() {
    FizzBuzz fizzBuzz = FizzBuzz.builder().add("Fizz", 3).add("Buzz", 5).build();
    FizzBuzz buzzFizz = FizzBuzz.builder().add("Buzz", 5).add("Fizz", 3).build();

    assertEquals(fizzBuzz.getValue(15), buzzFizz.getValue(15));
}

-
A divisor is only added once:

For this case you need to deice how to handle it. Currently both strings will be output. Should the second call take precedence, be ignored, cause an exception?

@Test
public void BuilderAdd_DivosorAddedMultipleTimes_EXPECTED_RESULT() {
    FizzBuzz fizzBuzz = FizzBuzz.builder().add("Fizz", 3).add("Buzz", 3).build();

    // assert expected result 
}

Your test names are fairly generic. When a test fails the first thing you see is the test name. A good test name should be able to give you a good idea of what might be failing even before you see the test. Similarly, when someone is reading the test code for the first time, the shouldn't have to work to get a sense for the intent of the test.

Part of this issue is a result of your test cases being general categories. With this specific setup, try a number of things. Instead it is better to have many specific test cases instead of a few general test cases.

The biggest issue with doing this is that once one thing fails, no other assertions are executed. If there many things broken, you want to know about all of them. What you don't want is to find out that the thing you just fixed was only part of the problem and other things are still failing.

A good test case has three main sections: Arrange, Act, and Assert. In your test cases, you are asserting many unrelated things. Checking a multiple of one is one test. Checking a multiple of both is a different test. Checking a non-multiple of both is yet another test.

When you write your test cases like this, giving specific names becomes much easier. Look at the test case I added to show the error in your code. The name tells you:

• What is being tested.

• The context in which it is being tested.

• The expected result.

If this test fails, I can tell you exactly what is wrong without looking at the code of the test case. Any one seeing the code for the first time doesn't have to think about what was the intent of this specific assertion.

Of the test cases you have, then seem to follow that same pattern. For the most part, they only differ on the inputs. Just like with production code, repeated code in tests is a bad thing and can lead to errors.

You can use inheritance to create the core functionality to a set of tests that are similar. An abstract test class can implement a number of test cases. Then you create a subclass for each specific situation. Each subclass can implement a abstract method that produces the FizzBuzz to use in each test case.

For tests when the test cases are testing the same thing, but need to try many different inputs values, you can use parameterized test. This is one good way to avoid the issue of having the first failure prevent other assertions from being executed.

StackExchange Code Review Q#74524, answer score: 15