Sorry for the delay in this week's blooper solution. On Monday we saw this code:
boolean hasThing( Listthings, Thing thing ) { for ( int i=0; i < things.size(); i++ ) { if ( thing.equals( things.get( i ) ) ) { return true; } } return false; }
Can we simplify this? Sure. It pays to know the class libraries well, and the collections framework in Java in particular:
boolean hasThing( Collectionthings, Thing thing ) { return things.contains( thing ); }
As well as being more compact, this idiom is safer if the collection is a synchronized wrapper (as pointed out by reader alex in the comments). We were also able to specify the things parameter as a Collection rather than a List, following item 34 in EJ ("Refer to objects by their interfaces"), since we no longer need the ability to iterate items in the collection by index, which may well have been inefficient depending on whatever implementation of list was passed to us.
Blog readers jeff and Mike Kaufman also pointed out that this code doesn't check parameters for validity. Specifically, if this is a public method, it should check if the two parameters are null, or document the behavior when they are null in its javadoc.
Indeed, the whole method itself is very likely to be unnecessary in this particular example. But only because I super-simplified the original code, which of course contained some side effects. Joy :)
Back on Monday with another blooper...
Its Monday! Next Blooper! Next Blooper! Next Blooper!
ReplyDelete