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

Generic sum method for a Collection<Number>

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

Problem

While answering this question, I wanted to use a generic sum of Collection function. Java doesn't have a native method, so I wrote one. There are a couple things I don't like about it though, so I wrote a more specific method for my answer.

public static > BigInteger sum(U numbers) {
    BigInteger sum = BigInteger.ZERO;

    for ( T number : numbers ) {
        sum = sum.add(new BigInteger(number.toString()));
    }

    return sum;
}


The first problem of course is that this wouldn't work for all Number types. In particular, Float, Double, and BigDecimal are not guaranteed to fit in an integer type. That's easy enough to fix:

public static > BigDecimal sum(U numbers) {
    BigDecimal sum = BigDecimal.ZERO;

    for ( T number : numbers ) {
        sum = sum.add(new BigDecimal(number.toString()));
    }

    return sum;
}


Now the problem is that it switches from an integer type to a decimal type. So we have to convert it back if we really preferred an integer type.

I'm also not happy with converting to and from a String to convert it to a BigDecimal. They're all numeric types. Why add this intermediate step? Unfortunately, String is the only interoperable type. All the Number types have a toString method, and BigDecimal has a constructor from a String.

Overall, this may be more hacky than useful. Does anyone have an alternative version that works better? Or should I stick with writing a custom method each time the issue arises? Or am I being too hard on this version?

Solution

Choosing between Long/Integer and Float/Decimal

I'd let the user choose.
Not because that's the best way, but because I don't see any other way.

Number itself is not reflective on the presence of decimal digits.
It would need an interface Integer and interface Decimal or something like that in order to deal with this in a single signature.
However, the API is not designed this way.

So, if both is needed, I'd have a sumInt and a sumFloat method, or maybe sumInteger and sumDecimal. (I prefer Float over Decimal because Decimal is simply wrong because it means 1/10ths and that's just the wrong name. BigDecimal is the wrong name.)

Simpler usage of Generics

The type parameter U is redundant. You could directly go for this signature:

public static  BigDecimal sum(Collection numbers)


And given that inside you actually don't need T, Number is sufficient, you could go for this signature:

public static BigDecimal sum(Collection numbers)


Going via String is a bit "far away" from numbers, and unnecessary. You could use Number.longValue() in case of BigInteger and Number.doubleValue() in case of BigDecimal.

Then the code would look like this:

public static BigInteger sumInt(final Collection numbers) {
    BigInteger sum = BigInteger.ZERO;
    for (final Number number : numbers)
        sum = sum.add(new BigInteger(number.longValue());
    return sum;
}

public static BigDecimal sumFloat(final Collection numbers) {
    BigDecimal sum = BigDecimal.ZERO;
    for (final Number number : numbers)
        sum = sum.add(new BigDecimal(number.doubleValue());
    return sum;
}


How it looks like with Java 8 Streams

With Java 8 Streams, the code could look like this:

public static BigDecimal sumFloat(final Collection  numbers) {
    return numbers
            .parallelStream()
            .mapToDouble(Number::doubleValue)
            .mapToObj(BigDecimal::new)
            .reduce(BigDecimal.ZERO, BigDecimal::add);
}


LSP Violation

However, all these implementations violate the LSP - Liskov Substitution Principle. They will not work correctly for all potential Number types. If the Collection numbers contains BigDecimal or BigInteger objects which exceed the range expressible in double or long, the functions would fail to produce the correct result.

Given the signature, which suggests that any Number would do, and given the fact that class BigInteger extends Number and class BigDecimal extends Number, and given the fact that the signature uses BigInteger resp. BigDecimal as a return value, that would be a pretty unexpected behavior.

The following helper function can fix that problem for the known classes BigDecimal and BigInteger:

public static BigInteger toBigInteger(final Number number) {
    return
        Number instanceof BigInteger ? (BigInteger) number :
        Number instanceof BigDecimal ? ((BigDecimal) number).toBigInteger() :
        new BigInteger(number.longValue());
}


However, if someone creates another subclass of Number which exceeds the range of long, there still would be a problem. This problem could only be solved by fixing the API: class Number should have toBigInteger() and toBigDecimal() methods, which in Number, could be implemented with corresponding defaults. Because the root cause for the LSP violation is not in your code, not in my code, but in the API - in the way how class Number actually is defined.

Using the helper function, the code could look like this:

public class Numbers {

    public static BigInteger sumInt(final Collection numbers) {
        BigInteger sum = BigInteger.ZERO;
        for (final Number number : numbers)
            sum = sum.add(toBigInteger(number);
        return sum;
    }

    public static BigDecimal sumFloat(final Collection  numbers) {
        return numbers
                .parallelStream()
                .map(Numbers::toBigDecimal)
                .reduce(BigDecimal.ZERO, BigDecimal::add);
    }

    public static BigInteger toBigInteger(final Number number) {
        return
            Number instanceof BigInteger ? (BigInteger) number :
            Number instanceof BigDecimal ? ((BigDecimal) number).toBigInteger() :
            new BigInteger(number.longValue());
    }
    // ...
}

Code Snippets

public static <T extends Number> BigDecimal sum(Collection<T> numbers)
public static BigDecimal sum(Collection<? extends Number> numbers)
public static BigInteger sumInt(final Collection<? extends Number> numbers) {
    BigInteger sum = BigInteger.ZERO;
    for (final Number number : numbers)
        sum = sum.add(new BigInteger(number.longValue());
    return sum;
}

public static BigDecimal sumFloat(final Collection<? extends Number> numbers) {
    BigDecimal sum = BigDecimal.ZERO;
    for (final Number number : numbers)
        sum = sum.add(new BigDecimal(number.doubleValue());
    return sum;
}
public static BigDecimal sumFloat(final Collection<? extends Number>  numbers) {
    return numbers
            .parallelStream()
            .mapToDouble(Number::doubleValue)
            .mapToObj(BigDecimal::new)
            .reduce(BigDecimal.ZERO, BigDecimal::add);
}
public static BigInteger toBigInteger(final Number number) {
    return
        Number instanceof BigInteger ? (BigInteger) number :
        Number instanceof BigDecimal ? ((BigDecimal) number).toBigInteger() :
        new BigInteger(number.longValue());
}

Context

StackExchange Code Review Q#78239, answer score: 6

Revisions (0)

No revisions yet.