friendly-coins Rule #3: Wrap all primitives and Strings.
This was rather invasive. The motive behind it (this is from Jeff’s article) is to add semantics to the use of atomic values; and also (a side benefit), give you a chance to stick behavior on those “small objects”.
In the Friendly Coins problem, it’s fairly clear there are at least three distinct flavors of numbers (all integers, by the way):
- A coin has a Denomination (e.g., a nickel has a denomination of 5 cents).
- When you are making change, you will then have a certain number of each Denomination. I decided to call this flavor of number a Cardinality.
- Finally, there is the number which arises by combining a bunch of Denominations into a handful of change (which we hope is Friendly, of course). I decided to call this Money, because that’s what it is, after all.
Since we are dealing with small objects, I used the Instance Singleton pattern to make sure each Denomination and Cardinality only ever had one instance in memory (I decided NOT to do this for Money, since Money is only ever persisted in memory about once per value in the solution, and Money, unlike the others, is mutable – and I didn’t even want to think about implementing copy-on-write).
Just a small taste:
BEFORE:
public static CoinSet createCoinSet(Integer... denominations) {
return new CoinSet( denominations );
}
...
int sum = 0;
...
private void incrementDenomination( int denomination ) {
int card = getCardinality( denomination );
denominations.put( denomination, 1 + card );
sum += denomination;
}
AFTER:
public static CoinSet createCoinSet(Denomination... denominations) {
return new CoinSet( denominations ) ;
}
...
private Money sum = new Money();
...
private void incrementDenomination( Denomination denomination ) {
Cardinality card = getCardinality( denomination );
setCardinality( denomination, Cardinality.getInstance( card.intValue() + 1 ));
sum.addCoin( denomination );
}
One of the choices I made when encapsulating the ints: since Integer cannot be extended, I could either add methods to my own spanky new WrappedInteger base class, or provide static helper methods. If one writes helper methods, one immediately gets into the game of exposing the internal value (oops, there’s a line of code or a dot or whatever); manipulating it (a call to a helper); then possibly re-encapsulating it. So, I gave WrappedInteger the needed comparison and plus/minus methods.
So, I recompile, and LOOK: I get warnings to the effect that I am comparing nonconvertible classes when I do this:
if( total.equals( denomination ) )
… Happily, this is exactly what I want to do – if the total change to give is equal to the denomination – then it’s a single coin. But in general, comparing denominations and Money would seem to be a no-no (since Denominations aren’t really money; they’re just the abstract concept of the possibility of representing a Money. You need a Denomination and a Cardinality to make Money). So to avoid the warning, I do this:
if( new WrappedInteger( total ).equals( denomination ) )
In fact, this is exactly what I want – normally, comparing apples to oranges is not good. In this case, all I needed to know was whether the apple weighed exactly the same. Or, was the same color. Or, something. You know what I mean.
In my case I’m not sure I’ve done things in a complete optimal way – I got rid of the messiness of helper methods, but have some warnings in the implementations, and try as I might, I simply cannot get my WrappedInteger implementation to not exceed the 50-line limit (!)
I’m sitting here wondering: If Java had automatic casting (as does, for example, Scala), or allowed me to extend Integer, then instead of implementing my own arithmetic and comparison methods, I’d just use the out-of-the-box stuff, and everyone would be happy. Boo Java. Or, boo on me, if there’s some simple thing I should have done but have missed. On the other hand, the exercise of designing my own general numeric classes and specializations was valuable in its own right …
Less Mundane stuff …
In addition to the numeric types, I played with the StringJoin helper method, introducing Joiners and Joinees.
BEFORE:
public static final String EMPTY_STRING = "";
...
public static String stringJoin( Collection objects, String joiner ) {
StringBuilder builder = new StringBuilder();
String useJoiner = EMPTY_STRING;
for( Object o : objects ) {
stringJoinAppend( o, useJoiner, builder );
useJoiner = joiner;
}
return builder.toString();
}
private static void stringJoinAppend( Object o, String joiner,
StringBuilder builder ) {
builder.append( joiner );
builder.append( (null == o) ? "null" : o.toString() );
}
AFTER:
public static final Joiner EMPTY_STRING = new Joiner("");
...
public static String stringJoin( Collection objects, Joiner joiner ) {
StringBuilder builder = new StringBuilder();
Joiner useJoiner = EMPTY_STRING;
for( Object o : objects ) {
stringJoinAppend( new Joinee(o), useJoiner, builder );
useJoiner = joiner;
}
return builder.toString();
}
private static void stringJoinAppend( Joinee o, Joiner joiner,
StringBuilder builder ) {
builder.append( joiner );
builder.append( o.toString() );
}
Aside from giving me a place to put the constant Joiner.COMMA (commonly used, of course), and the nicety of having the translation of the null value to the string "null" be in the Joinee class, I question the use/smell of this particular change, because in the end, the implementation is twice the length of its original (I didn't show you the Joiner and Joinee classes - they're rather mundane). Maybe there would be future methods that also use Joiner and Joinee, making it all worthwhile. On the other hand, its usage (as opposed to its implementation) is virtually identical to before the change; the only thing you have to do is send the stringJoin method a Joiner rather than a plain String (the Collection sent to it is automatically converted itemwise into Joinees).