JavaBlogging

Today a story about a design decision whether or not to implement the Comparable interface. We will start with the following code snippet – a simplified version of a UI widget that holds info about a person and can be selected by the user:

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:

public static class SelectablePerson {
    private final String name;
    private boolean selected;

    public SelectablePerson(String name) {
        this.name = name;
        this.selected = false;
    }

    public boolean isSelected() {
        return selected;
    }

    public void setSelected(boolean selected) {
        this.selected = selected;
    }
}

A quick notice: the code above is just to show my point in this post, not to be taken seriously when implementing similar functionality As you can see this class is fairly simple: just with a name and ’selected’ variable that can be switched on and off. We use those objects in our application to show a list of persons (maybe employees?) and allow the user to select a subset of them (to give a raise or fire?).

All is working fine, but then we get a new requirement: since it is hard to find an employee on the list we want to have it now sorted alphabetically. No biggy, right? We can sort all our objects with Collections.sort() and display them sorted. There is only one decision to make: either we will have ‘SelectablePerson’ implementing Comparable interface or write a separate comparator and use it in sort. So which one is the best option? Is it even important? Let’s check that!

Assume we go with the first possibility and implement additional interface. This means that we have to add one new method to the class – compareTo()… right? Well, close but not really. If we have compareTo() we also need to implement equals(). If we have this one we also need hashCode(). After all the class will look like this:

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:

public class SelectablePerson implements Comparable<SelectablePerson> {

    // ...

    public int compareTo(SelectablePerson other) {
        return this.name.compareTo(other.name);
    }

    @Override
    public int hashCode() {
        return name.hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        if (!(obj instanceof SelectablePerson)) return false;
        SelectablePerson other = (SelectablePerson) obj;
        return this.compareTo(other) == 0;
    }
}

It is a pain that we had to go with equals() and hashCode() – I mean why do you need a hash code to sort out widgets? This is an indication that something is going wrong… but besides that all is fine and we can do the sorting. Mission accomplished!

Next requirement: we need our widget to react on the changes in other widgets of UI. This is done in a standard way with Observer-Observable pattern. What’s awesome is that in Java implementing that is really easy. Let’s be more specific with the new requirement: we need to have a reset button that will unselect all of the SelectablePerson widgets. This is the code of the button:

1:
2:
3:
4:
5:
6:

public class ResetButton extends Observable {
    public void press() {
        setChanged();
        notifyObservers();
    }
}

Now a simple change in SelectablePerson – implement Observer and as a reaction on (any) change of an observed object clear the selection. The code is following:

1:
2:
3:
4:
5:
6:
7:
8:
9:

public class SelectablePerson implements Comparable<SelectablePerson>,
        Observer {

    // ...

    public void update(Observable observable, Object value) {
        selected = false;
    }
}

Now when an reset() method of a reset button is invoked all SelectablePerson objects observing it will be unselected. Nice, huh?

Now we are back to the main question: was the decision of implementing Comparable good? You can use the code above, compile it and test it… all is working, right? Well, no… I really suggest trying to implement it as the bug is not that obvious and its good to see it on your own. One of the use cases making the bug appear is when you will instantiate the widgets with the following names: “Smith”, “Brown”, “Smith”, “Jackson”. You will notice that sorting is working fine, but the reset button does not work correctly – it resets all except one of the “Smith” widgets. Strange, huh? Even more bizarre: why only one of the same widgets is affected??

The explanation is quite simple: the reset button extends Observable (as it should). The Observable class internals handle the keeping and managing of all the objects that observe it. More specifically it keeps a list (Vector) of all observing objects and (when appropriate) notifies them about change in observed object. Every time you add another observer the Observable.addObserver() code checks whether or not the provided object is already in the list. How does it check? By comparing with equals() function, the one that we had to override… So because the name “Smith” appears twice in UI both of its widgets are considered by reset button equal and therefore only one is added to the list of observers. And the one that is not added is the one malfunctioning…

Now back to the main topic: would we encounter the same bug if we wrote a Comparator instead of implementing Comparable? No, we would not. You might argue that there are ways of keeping the Comparable and removing the bug… true, but that would just mask the design error we made. The reason we run into trouble is that the SelectablePerson instances are not suppose to have equals() or compareTo() methods as they are in fact unique and representing UI entities rather than values they carry. This comes to an additional lesson: if you do not really need to implement Comparable, do not do it… especially if you can get by with a Comparator