JComboBox setSelectedItem Bug or Feature?

Question

the Original code of set selected Item is:

public void setSelectedItem(Object anObject) {
    Object oldSelection = selectedItemReminder;
    Object objectToSelect = anObject;
    if (oldSelection == null || !oldSelection.equals(anObject)) {

        if (anObject != null && !isEditable()) {
            // For non editable combo boxes, an invalid selection
            // will be rejected.
            boolean found = false;
            for (int i = 0; i < dataModel.getSize(); i++) {
                E element = dataModel.getElementAt(i);
                if (anObject.equals(element)) {
                    found = true;
                    objectToSelect = element;
                    break;
                }
            }
            if (!found) {
                return;
            }
        }

in my opinion the line

 if (anObject.equals(element)) {

should be

 if (element.equals(anObject)) {

Consider a Combo box displaying eg. languages then you hav a class like

class Language {
     String code; // eg. "en"
     String name; // eg. "English"
...
}

if you add Language items to your ComboBox the toString function is used to Display an Item. In the above class the toString function would return the name. A call setSelectedItem("en") fails because

String.equals(Language) will fail because Language.toString() will return "English"

the other way round Language.equals(String) would be helping because the class Language could override

boolean equals(String comp) {
   return comp.equals(code)
}

Just for clarification, I know how to create a Combobox with the desired behaviour, my question is: is the comparison in the original code a bug or did I miss something fundamental?

Answer 1

The properly implemented Object.equals is symmetric, meaning that there should be no difference between anObject.equals(element) and element.equals(anObject).

You are describing a situation where the combobox model contains objects of type Item, but you want to select select an item by specifying an object of type Prop, where the value of Prop describes some property of Item.

Using technically incorrect implementation of equals() method you can select a combobox item by passing an instance of Prop instead of Item.

With the original code, you will have to provide broken equals() implementation in Prop class, and with your modification you will have to provide broken equals() implementation in Item class. If the Prop is some library class (as String in your example) then the former case is, of course, impossible, and i assume that the reason for your proposed modification is to allow the latter case.

I am not sure that library creators tried to prevent programmers from implementing broken equals() by choosing that specific anObject.equals(element) expression, but even if it was element.equals(anObject) it would still be bad practice to provide deliberately incorrect equals() implementation just for the sake of simplifying the combobox selection.

The proper way to perform selection by property would be to search the combobox data for the item with the required properties or to create a completely new instance of Item with the desired properties and then pass that item into the setSelectedItem.

If you are lucky to already use Java 8 then selecting the required item from a list is one-liner, and if not then you will have to write some boilerplate code with cycle, but at least you will have the proper equals implementation and clean conscience.

Answer 2

To override the inherited equals method, you should pass an object as parameter and not String

public boolean equals(Object obj){
//code goes here
}