HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Let's Break Down the Party by Breaking the Party Down Recursively

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thepartybreakingrecursivelydownletbreak

Problem

This is a rags-to-riches an overdue alternative implementation on my previous question: Let's Break Down the Party

This alternative uses the int internal representation suggested by @janos and recursion to arrive at the same results. For starters, here are the changes in Denomination enum to support the internal representation:

public enum Denomination {
    // values;

    static final int MULTIPLIER = 100;

    private final BigDecimal value;
    private final int centValue; // internal representation
    private String description;

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

    /**
     * @return the value in cents
     */
    public int getCentValue() {
        return centValue;
    }

    ...

}


(I've kept the original BigDecimal value parts of code intact since I am treating the new internal representation as an addition rather than a replacement to the original)

MULTIPLIER_VALUE uses the default (package) access modifier as it is only meant to be visible within the same package, i.e. the Calculator:

```
public class Calculator {

/**
* Break down the input into {@link Denomination} values by recursion.
*
* @param input the value to break down
* @return an unmodifiable {@link Map} with the {@link Denomination} as keys and a
* positive integer, the multiplier, as values
*/
public static Map getBreakdownRecursively(double input) {
return Collections.unmodifiableMap(
recurse(Stream.of(Denomination.values()).iterator(),
(int) (input * Denomination.MULTIPLIER),
new EnumMap<>(Denomination.class)));
}

private static Map recurse(Iterator iterator,
int input, Map result) {
if (input == 0 || !iterator.has

Solution

I decided to introduce a quantize() method on Denomination to do the multiplication:

public static int quantize(double input) {
    return (int) (input * MULTIPLIER);
}


This changes the Denomination's constructor:

this.centValue = quantize(value);


And the helper method itself:

public static Map getBreakdownRecursively(double input) {
    return recurse(Stream.of(Denomination.values()).iterator(),
            Denomination.quantize(input), new EnumMap<>(Denomination.class));
}


As hinted from above, I moved the Collections.unmodifiableMap() call to the last recursion within the recurse() method:

private static Map recurse(Iterator iterator,
        int input, Map result) {
    if (input == 0 || !iterator.hasNext()) {
        return Collections.unmodifiableMap(result);
    }
    Denomination current = iterator.next();
    ...
}


Now, the original approach described in the question works well and looks simple enough, but what if I went a little deeper with Java 8 features? Looks like a nice place to introduce Map.computeIfAbsent()...

private static Map recurse(Iterator iterator,
        int input, Map result) {
    if (input == 0 || !iterator.hasNext()) {
        return Collections.unmodifiableMap(result);
    }
    Denomination current = iterator.next();
    return recurse(iterator, result.computeIfAbsent(current, key ->
                Optional.of(Integer.valueOf(input / key.getCentValue()))
                    .filter(i -> i.intValue() != 0).orElse(null)) == null ? 
                        input : input % current.getCentValue(), result);
}


Sure, readability is sacrificed, but hey, just one return statement! By taking advantage of computeIfAbsent() and its return type, I can optionally store the output of the current iteration's breakdown (when the quotient is not zero), and determine what should be the next value to recuse with (the original input, or the modulus).

edit

I suppose I can improve the readability somewhat by this...

private static Integer getValue(int input) {
    // return Optional.of(Integer.valueOf(input))
    //                 .filter(i -> i.intValue() != 0).orElse(null);
    return input == 0 ? null : Integer.valueOf(input);
}


(fixed that)

The return statement will then read:

private static Map recurse(Iterator iterator,
        int input, Map result) {
    if (input == 0 || !iterator.hasNext()) {
        return Collections.unmodifiableMap(result);
    }
    Denomination current = iterator.next();
    return recurse(iterator, result.computeIfAbsent(current, key ->
                getValue(input / key.getCentValue())) == null ? 
                        input : input % current.getCentValue(), result);
}

Code Snippets

public static int quantize(double input) {
    return (int) (input * MULTIPLIER);
}
this.centValue = quantize(value);
public static Map<Denomination, Integer> getBreakdownRecursively(double input) {
    return recurse(Stream.of(Denomination.values()).iterator(),
            Denomination.quantize(input), new EnumMap<>(Denomination.class));
}
private static Map<Denomination, Integer> recurse(Iterator<Denomination> iterator,
        int input, Map<Denomination, Integer> result) {
    if (input == 0 || !iterator.hasNext()) {
        return Collections.unmodifiableMap(result);
    }
    Denomination current = iterator.next();
    ...
}
private static Map<Denomination, Integer> recurse(Iterator<Denomination> iterator,
        int input, Map<Denomination, Integer> result) {
    if (input == 0 || !iterator.hasNext()) {
        return Collections.unmodifiableMap(result);
    }
    Denomination current = iterator.next();
    return recurse(iterator, result.computeIfAbsent(current, key ->
                Optional.of(Integer.valueOf(input / key.getCentValue()))
                    .filter(i -> i.intValue() != 0).orElse(null)) == null ? 
                        input : input % current.getCentValue(), result);
}

Context

StackExchange Code Review Q#88238, answer score: 5

Revisions (0)

No revisions yet.