Thoughts on collections

Yesterday I stumbled upon code like the following:

if (coll != null) {
   for (Item item : coll) {
      // do something
   }
}

This pattern was repeated over and over. I wondered:
“Why on earth can this collection be null at all?” – The succinct answer can be found in the definition of the field:

private Collection<Item> coll;

This field is never initialized during the enclosing object’s instantiation.
Alright, this is not really bad by itself. So I will insert a null test every time I am accessing the field and everything will be fine.

STOP -> Wrong (implicit) question! (“What’s wrong with it?”)

You can of course answer this question, but it would be better to ask: “What’s the use of omitting the initialization?” answer: “Less memory.”

STOP -> I’m asking for a hearing device. Did my opponent really reason about memory? In a programm that is parsing XML via SAX, writes the resulting data into a database, reading it again from there and finally builds a DOM, which is written to the filesystem?

So what is it good for to omit the initialization? – Nothing but unreadable code and possible errors. Therefore collections should always be initialized with an appropriate empty collection.

To me there is no semantical difference between an empty collection and null.
If there is nothing in it, there’s nothing in it. I do not need different “nothings”.

IS there anything left to say? -> “YES!”

Why is the collection declard as Collection and not as an appropriate subtype? – Doesn’t the developer know what he wants?
A quick screening of the code shows: the collection is always created as ArrayList. Then it is obvious to declare the field as List<Item>. After all, List is more powerful than Collection.
Needless to say that a closer look at the code found parts where the field’s null check was omitted…

We then performed the initialization and removed every null check. We avoided changing Collection to List at that time:

private Collection<Item>; coll = new ArrayList<Item>();

And we have been really lucky: There have been a few unittests. We ran them and: NullPointerException when accessing coll! – I was slightly shocked! – rather: I pretended to be.

The cause lay here:

public void setItems(List<Item> items) {
   coll = items;
}

Looks completely innocent, but herewith you can set coll to null. So quickly searching all senders of this method and found (here list is a local variable; please don’t ask for the rest of the method…):

if (list == null || list.isEmpty()) {
   thing.setItems(null);
}

That’s the place where null is creeping back in through the backdoor. The first step was to always initialize list. For a local variable this is trivial. Side effect: less code:

thing.setItems(list);

Similar places were changed accordingly.

I often wonder: “Who the hell needs setters for collection fields?”
Is there an alternative?
Once you know it, it is quite obvious:
The collection-field is an implementation detail, managing a certain number of elements. Probably you want to add or remove elements. The actual process must be controlled by the owner of the collection-field. Therefor:

public void addItem(Item item) {
   coll.add(item);
}

public void removeItem(Item item) {
   coll.remove(item);
}

Of course you only need these methods, if you need to modify the collection this way.

Finally: ACCESS

To protect the above methods (add/remove) from the use of backdoors, mutable access to the collection field is to be prohibited. Your compiler can help:

public List<Item> getColl() {
   return Collections.unmodifiableList(coll);
}

Conclusion:

Collections in objects need protection and care:

  • They must never be(come) null
  • The enclosing object is responsible. All changes must be performed by it

The you’ll get:

  • Straightforward code, because many error-prone situations are impossible (except reflection-calls…)
  • Easier traceability of the changing code locations

I hope my words have been clear-cut, reasonable and entertaining.

2 thoughts on “Thoughts on collections

  1. Goot article! You are using Collection as a class member. Additionally I would move this “plain” collection into its own class. This will make the interface smaller for the colleciton (replace inheritance with delegation). The upper class (Collection) will become a new instance variable of your customized Collection-Class (“YourItemColleciton”).

    • Hi Matthias! Thanks for the comment. What is the point in wrapping the collection inside a new class? In my example the only uses of the collection are the methods in the article: addItem, removeItem, getColl. Inside the class the only uses are iterations. Could you explain in more detail what you meant? Especially the “delegation for inheritance”.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s