My "anti-patterns" - 3

After the previous entry with rather bombastic statements ("don't use inheritance!") this is going to be a rather modest and specific one.

Pair

It looks that almost in every project there's someone who once wrote this:

public class Pair<F,S> {
    private final F first;
    private final S second;
    public Pair(F first, S second) {
        this.first = first;
        this.second = second;
    }
    public F first() {
        return first;
    }
    public S second() {
        return second;
    }
}

When I first saw this, I thought "if it's so useful, why doesn't Java have it already?" It didn't occur to me then, but after some time I got to the conclusion why. Not only is this class useless, it actually does harm.

Most obviously, it obscures the code. pair.second() - what is it? What this statement is talking about?! Who's on first? What's on second? Things should be referenced by their meaning, not by a position inside a structure! What is it? Assembly language? [SP+8]?!

Secondly, I believe that it actually hides the reluctance of a developer to name things and think about them. It's much easier to hide behind an anonymous "pair" than to think whether this "alliance" between two objects is really needed and how it should be called.

Sometimes it is also a quick way to circumvent the restriction that a method has to have only one return value. Need a second one? (Most probably an additional flag.) Make it return a pair - the original object plus a boolean! What's the meaning of this boolean? Don't bother, it's "second"! Good luck in reverse engineering what this code does!

Recently I happened to see the following "masterpiece":

HashMap<String, Pair<Integer, HashMap<String,
    Pair<String, String>>>>

Notice this "operator" at the end? Amazing how robust the Java compiler is!

By the way, the above example reminds me of another idea (and this probably deserves a topic of its own). I'm starting to think that collections, especially maps, should probably never be used per se, in particular when being used as return values. They should rather be wrapped into classes whose name describe the meaning of the collection, not its type signature. Don't be alarmed, in most of the cases the wrapper class will have to expose only a very small subset of the methods of the collection. However, they would have a chance to have much better names, explaining what purpose the collection serves in this particular context. For instance, if you have a mapping from "logical" names of something to their "physical" names (whatever it means), a code like namesMapping.getPhysicalName("Attr1") really looks much nicer than namesMapping.get("Attr1").

Back to the "pair". Whenever one uses this "pattern", a duplication of its declaration is inevitable. Imagine a method that returns a pair. The description of the pair (its two types) will first be declared at the signature of the method, another time at the return statement of the method (accompanied with the new keyword) and every time this method is used (most probably the return value would be stored in a local variable). So the same combination of types is being repeated all over the place, cluttering the code.

Hopefully I convinced you to avoid it. If not, think of a more advanced class - a triple!

Added on Nov 4, 2010: The world has definitely gone crazy: http://www.javatuples.org/. As a response, see also http://thedailywtf.com/Articles/Overloaded-Help.aspx.