Let's Break Down the Party

This is a rags-to-riches question, the original is here: Coin dispenser program

There are three parts to this, a Denomination enum, a Calculator utility class and the unit test for it, using TestNG.

Please feel free to comment on any parts of the code, including comments.

Denomination enum

```
import java.math.BigDecimal;
import java.util.Objects;

public enum Denomination {
A_MILLION(1_000_000, "$1 million"),
FIFTY_DOLLARS(50, "$50"),
TWENTY_DOLLARS(20, "$20"),
TEN_DOLLARS(10, "$10"),
FIVE_DOLLARS(5, "$5"),
DOLLAR_NINETY_NINE(1.99, "$1.99"),
A_DOLLAR(1, "$1"),
QUARTER(0.25, "25¢"),
DIME(0.1, "10¢"),
NICKEL(0.05, "5¢"),
A_CENT(0.01, "1¢");

private final BigDecimal value;
private String description;

private Denomination(double value, final String description) {
this.value = BigDecimal.valueOf(value);
this.description = Objects.requireNonNull(description);
}

/**
* @param input the value to compare against
* @return true if input is not smaller than the current value
*/
public boolean canBreakdown(double input) {
return BigDecimal.valueOf(input).compareTo(value) >= 0;
}

/**
* Breaks down the input against the current value.
*
* @param input the input to start
* @return a two-element array, the first being the quotient (aka multiplier) and the second
* being the remainder
*/
public double[] breakdown(double input) {
final BigDecimal[] results = BigDecimal.valueOf(input).divideAndRemainder(value);
return new double[] { results[0].doubleValue(), results[1].doubleValue() };
}

@Override
public String toString() {
return description;
}

/**
* @param multiplier the value to represent
* @return a representation of the multiplier and the current value
*/
public String toString(int multiplier) {
return String.format("%d x %s", Integer.v

The internal representation of denominations

Just as I explained for the question you linked,
I don't see the reason to use BigDecimal to represent the denominations.
I think it would be better to make cents the base unit,
and use an int or long as the internal representation.

Here's Denominator, rewritten to use int as the internal representation.
I preserved the original API, so Calculator and CalculatorTest work with this unchanged (I cut out the JavaDoc for brevity). Notice the use of a MULTIPLIER, and its explanation in a comment.

enum Denomination {
    A_MILLION(1_000_000, "$1 million"),
    FIFTY_DOLLARS(50, "$50"),
    TWENTY_DOLLARS(20, "$20"),
    TEN_DOLLARS(10, "$10"),
    FIVE_DOLLARS(5, "$5"),
    DOLLAR_NINETY_NINE(1.99, "$1.99"),
    A_DOLLAR(1, "$1"),
    QUARTER(0.25, "25¢"),
    DIME(0.1, "10¢"),
    NICKEL(0.05, "5¢"),
    A_CENT(0.01, "1¢");

    private final int value;
    private String description;

    // Set to 10^k where k = maximum number of decimals to support in denominations
    // For example, to support 0.01 (1 cent), set to 10 ^ 2 = 100
    private static final int MULTIPLIER = 100;

    private Denomination(double value, final String description) {
        this.value = Double.valueOf(MULTIPLIER * value).intValue();
        this.description = Objects.requireNonNull(description);
    }

    public boolean canBreakdown(double input) {
        return MULTIPLIER * input >= value;
    }

    public double[] breakdown(double input) {
        int intValue = Double.valueOf(MULTIPLIER * input).intValue();
        int div = intValue / value;
        int remainder = intValue % value;
        return new double[] { div, (double) remainder / MULTIPLIER };
    }

    @Override
    public String toString() {
        return description;
    }

    public String toString(int multiplier) {
        return String.format("%d x %s", multiplier, toString());
    }

    public double multiply(int multiplier) {
        return (double) value * multiplier / MULTIPLIER;
    }    
}

2-element double[] ?

It's always smelly when a method returns an array of n elements.
Since the size of the array is not part of the type,
the compiler cannot enforce that the array will really contain n elements.

This is especially awkward when the two values are not really the same type.
As is the case in Denomination.breakdown.
The method returns a 2-element double[],
but the first element is actually supposed to be an int.
Putting the int in the double[] is easy enough,
but it puts a serious burden on callers,
who now have to convert the first element to int if they want to use it.

It would be better to create a custom class for the return value,
and properly enforce the types:

public static class Breakdown {
    final int count;
    final double remainder;

    public Breakdown(int count, double remainder) {
        this.count = count;
        this.remainder = remainder;
    }
}

Then the breakdown method becomes:

public Breakdown breakdown(double input) {
    int intValue = Double.valueOf(MULTIPLIER * input).intValue();
    int count = intValue / value;
    int remainder = intValue % value;
    return new Breakdown(count, (double) remainder / MULTIPLIER);
}

And the caller can be simpler as well:

public static Map getBreakdown(double input) {
    final Map result = new EnumMap<>(Denomination.class);
    double temp = input;
    for (final Denomination current : Denomination.values()) {
        if (current.canBreakdown(temp)) {
            Denomination.Breakdown breakdown = current.breakdown(temp);
            result.put(current, breakdown.count);
            temp = breakdown.remainder;
        }
    }
    return Collections.unmodifiableMap(result);
}

Naming

In the getBreakdown method mentioned in the earlier point,
look how much more readable it becomes if I rename temp to remainder and current to denomination:

public static Map getBreakdown(double input) {
    final Map result = new EnumMap<>(Denomination.class);
    double remainder = input;
    for (final Denomination denomination : Denomination.values()) {
        if (denomination.canBreakdown(remainder)) {
            Denomination.Breakdown breakdown = denomination.breakdown(remainder);
            result.put(denomination, breakdown.count);
            remainder = breakdown.remainder;
        }
    }
    return Collections.unmodifiableMap(result);
}

Weak "API" in CaseBuilder

The CaseBuilder class has some methods with some overlap in logic:

• createEmpty: create an empty, unmodifiable builder

• create: create a builder with some denomination

• with: add denominations

The overlap is in create and with.
The semantic rule is that you start with create and then add more denominations using with.
Another semantic rule is that createEmpty is actually unmodifiable:
you cannot start with createEmpty and add more items using with later.

It wou

StackExchange Code Review Q#82760, answer score: 10