CodeSOD: A Fairly Generic Comparison to Make

Some languages support operator overloading. Others, like Java, do not. There are good arguments on either side for whether or not you should support the feature, but when you don't, you need an easy way to get the effects of operators without the syntactic sugar.

For example, if you have a custom class, and you need a way to perform comparisons on them, you may choose to implement the Comparator interface. This allows you to add some kind of comparison to an object without actually touching the object's implementation. For example, you might do something like:

class TransactionDateComparer implement Comparator<Transaction> { @Override public int compare(Transaction t1, Transaction t2) { return t1.transDate.compareTo(t2.transDate); } }

You can now pass objects of this type off to SortedSets, Arrays.sort, or any other situation where you want to order transactions. And, since Comparator is a functional interface, you can even just use a lambda instead, e.g.:

Arrays.sort(transactions, (Transaction t1, Transaction t2) -> t1.compareTo(t2));

There: you now know more about generics, interfaces, lambdas, and Java than Dave's co-worker. Because Dave's co-worker understood the need for comparators, but decided that implementing a new comparator for every new type that needs to be sorted was just too much work. So they did this instead:

public class GenericComparator implements Comparator<Object> { @Override public int compare(Object o1, Object o2) { if (o1 instanceof A) { … } else if (o1 instanceof B) { … } else if (o1 instanceof C) { … } else if (o1 instanceof D) { … } … } }

It's a good rule of thumb that if you're using an operator like instanceof, you've made a mistake somewhere. Like any rule, there are exceptions, but this is absolutely not one of them. They've abused generics to make a GenericComparator which generically only works on explicitly named classes. It's a mega-method, with one if branch for every domain class in their application. And it's used everywhere.

When Dave pointed out that this probably wasn't a good solution to their problem, the developer responsible countered that any time they needed to sort, they always knew what comparator type to use. "All the logic is encapsulated, which is very object oriented."

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published.