This might be considered too defensive but I don't really like the immutable collections because there is no compiler safety for misuse - only runtime errors (same goes for singleton, empty collections). The AbstractList implementation of add (i.e. throw new UnsupportedOperationException...) is a good example of a refused bequest code smell.
With hindsight the designers would probably have removed mutable methods from the java.util.Collection interface and introduced another interface/contract intended for use by concrete implementations concerned with mutability.
This issue can be compounded when working with third party code if you do not have access to sources so (without decompiling) you are forced to rely on sensible design or good documentation.
In addition I have seen code like this:
return(/* some condition */)?list:Collections.emptyList();
Whilst it is a slight memory optimisation use the empty list (instead of instantiating and returning empty list) it also creates a nasty trap for the caller. Worst case this leads to a game of Charades where all lists are wrapped as new lists "just in case" ... leading to bigger allocation/memory overheads.
So my general approach is to either:
Don't expose the list directly if it can be avoided.
If it really must be exposed then always return a clone.
That makes the behaviour safer and more predictable.
Saving fish by writing code! Applications developer in fisheries, specializing in webapps and moving 'enterprise-y' legacy systems to modern agile systems - Email or tweet me if you want to talk!
This might be considered too defensive but I don't really like the immutable collections because there is no compiler safety for misuse - only runtime errors (same goes for singleton, empty collections). The
AbstractList
implementation ofadd
(i.e.throw new UnsupportedOperationException
...) is a good example of a refused bequest code smell.With hindsight the designers would probably have removed mutable methods from the
java.util.Collection
interface and introduced another interface/contract intended for use by concrete implementations concerned with mutability.This issue can be compounded when working with third party code if you do not have access to sources so (without decompiling) you are forced to rely on sensible design or good documentation.
In addition I have seen code like this:
Whilst it is a slight memory optimisation use the empty list (instead of instantiating and returning empty list) it also creates a nasty trap for the caller. Worst case this leads to a game of Charades where all lists are wrapped as new lists "just in case" ... leading to bigger allocation/memory overheads.
So my general approach is to either:
That makes the behaviour safer and more predictable.
All good points and interesting things to consider