Wednesday, April 04, 2007

Composition over Inheritance... again

After what seemed like forever we have now finally begun to migrate our product from Java 1.4 to Java 5. Java 6 would have been nicer, but that is another story.

The first step was of course to switch compilers, runtime environment and Eclipse's source compliance settings. What followed were the usual suspects before we could compile again (uses of e. g. enum as a variable name, different package names in the XML area that bit us because of not using Factories etc).

However one thing did go unnoticed and was only found some days on a test system because of strange data-truncation warnings. For some reason negative numbers were too long by 1 digit. To understand this one has to know about a rather old data exchange format we need to support. It's string-based with leading zeros for numbers. Interestingly you can send a number like 99999 (positive), but only -9999 (negative), because there is a maximum length of 5 characters in the format.

The JDK's DecimalFormat almost does the job, but it will put a minus sign in front of the leading zeros, making the format too long by one character for negative numbers:

DecimalFormat tFormat = new DecimalFormat("00000") ;
// this is what it does
assert "99999".equals(tFormat.format(new BigDecimal("99999")));  // ok        
assert "-09999".equals(tFormat.format(new BigDecimal("-9999"))); // ok
assert "-00001".equals(tFormat.format(new BigDecimal("-1")));    // ok
// but this is what we want
assert "-9999".equals(tFormat.format(new BigDecimal("-9999")));  // fail
assert "-0001".equals(tFormat.format(new BigDecimal("-1")));     // fail

To achieve the desired behavior, one of the developers introduced a CustomNumberFormat class. This is it:

import java.math.BigDecimal;
import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.text.FieldPosition;
import java.util.Locale;

class CustomNumberFormat extends DecimalFormat {

    private int formatLength;

    private static final DecimalFormatSymbols SYMBOLS = new DecimalFormatSymbols(
            Locale.US);

    public CustomNumberFormat(String aFormat) {
        super(aFormat, SYMBOLS);
        formatLength = aFormat.length();
    }

    public StringBuffer format(double aNumber, StringBuffer aBufferToAppend,
                               FieldPosition aPos) throws NumberFormatException {
        StringBuffer tempResult = super.format(aNumber, aBufferToAppend, aPos);
        if (tempResult.length() >= 2 && tempResult.substring(0, 2).equals("-0")) {
            tempResult.deleteCharAt(1);
        }
        if (tempResult.length() > formatLength) {
            throw new NumberFormatException();
        }
        return tempResult;
    }

    public String formatNumber(BigDecimal aNumber) {
        if (aNumber == null) {
            return super.format(new BigDecimal("0"));
        }
        return super.format(aNumber);
    }

}

Apart from providing a convenience formatNumber(BigDecimal) method the most striking thing that you may notice is the overriden format(double, ...) method. Why would you override a method with a signature requiring a double value? Well, it turns out, that any other useful method in the super class is final or private, so no help there. However looking at the source of DecimalFormat reveals that calling the DecimalFormat.format(Object) method leads to this call-stack:

CustomNumberFormat.format(double, StringBuffer, FieldPosition) line: 22 
CustomNumberFormat(NumberFormat).format(Object, StringBuffer, FieldPosition) line: 216 
CustomNumberFormat(Format).format(Object) line: 133 

The second line is most interesting: It turns out that for BigDecimals the format(double, ...) gets called. Because this is overriden, the version in CustomNumberFormat is executed. It first calls super.format(double,...) and then removes the charAt(1) of the result, which must be a "0" if it starts with "-0". In case the number was too big, a NumberFormatException is thrown.

This achieves our goal:

 
DecimalFormat tFormat = new CustomNumberFormat("00000") ;
// this is what it does *and* what we want
assert "99999".equals(tFormat.format(new BigDecimal("99999")));  // ok        
assert "-9999".equals(tFormat.format(new BigDecimal("-9999"))); // ok
assert "-0001".equals(tFormat.format(new BigDecimal("-1")));    // ok

So far, so good, job done.

Enter Java 5.

Running the same code again - this time just for one example - leads to this:

DecimalFormat tFormat = new CustomNumberFormat("00000") ; 
String tResult = tFormat.format(new BigDecimal("-1"));
assert "-0001".equals(tResult): tResult; // java.lang.AssertionError: -00001

For some reason now the output is too long by one again. Great... must be a Java 5 bug, must it not?

Of course it is not. It is just the good old "Favor composition over inheritance" rule violated and showing its ugly face. The implementation of CustomNumberFormat relies on the execution path running through the format(double,...) method, i. e. on the concrete implementation of this runtime class library. Looking at the Java 5 source reveals that this is no longer the case for BigDecimals. Because of this, the overriden method never gets executed, effectively making it useless. Even worse: In a different scenario it might get called unintentionally, depending on how the implementation in the Java 5 class library was modified.

So, to solve the problem, just do as Josh Bloch tells you in Item 14 (link above): Favor composition over inheritance, i. e. do not extend DecimalFormat but simply use an instance of it like so:

 
import java.math.BigDecimal;
import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.util.Locale;

public class NewCustomNumberFormat {

    private DecimalFormat decimalFormat;

    private int formatLength;

    private static final DecimalFormatSymbols SYMBOLS = new DecimalFormatSymbols(
            Locale.US);

    public NewCustomNumberFormat(String aFormat) {
        decimalFormat = new DecimalFormat(aFormat, SYMBOLS);
        formatLength = aFormat.length();
    }

    public String format(Object aNumber) throws NumberFormatException {
        String tFromDecimalFormat = decimalFormat.format(aNumber);
        StringBuffer tempResult = new StringBuffer(tFromDecimalFormat);
        if (tempResult.length() >= 2 && tempResult.substring(0, 2).equals("-0")) {
            tempResult.deleteCharAt(1);
        }
        if (tempResult.length() > formatLength) {
            throw new NumberFormatException();
        }
        return tempResult.toString();
    }

    public String formatNumber(BigDecimal aNumber) {
        if (aNumber == null) {
            return format(new BigDecimal("0")).toString();
        }
        return format(aNumber).toString();
    }

}

While the semantics are the same, this class actually works in Java 1.4 and Java 5 and will most certainly continue to do so in later versions as well. Notice that this class does not extend anything, but merely utilizes an instance of DecimalFormat to delegate the formatting to, before cutting away one of the zeros, making it completely independent of DecimalFormat's implementation details.

While this is probably old news to many, the remaining few are probably worth the post :)

No comments: