Java Swing's JTree uses node's hashcode - why?

Question

It seems JTree has a -for me- unexpected behavior: it relies on the hashcode of the nodes instead of relying on its model. This deviates from the way e.g. JTable and JList work: these two allow you to provide some value via their models and then have a renderer to visualize it. JTree does not, and I would like to understand why (and if possible how to circumvent it).

Below an example, based on a JavaBean "person".

public class Person {
    
    String name;
    public String getName() {
        return this.name;
    }
    public void setName(String v) {
        this.name = v;
    }

    @Override
    public int hashCode() {
        // return super.hashCode();
        return java.util.Objects.hash(name);
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (obj == null) return false;
        if (getClass() != obj.getClass()) return false;
        Person other = (Person)obj;
        return java.util.Objects.equals(this.name, other.name);
    }

    @Override
    public String toString() {
        return super.toString() + "#" + System.identityHashCode(this)
                + ",name=" + name;
    }
}

And

public class JTreeTest {
    static public void main(String[] args) throws Exception {

        SwingUtilities.invokeAndWait(() -> {

            Person personCarl = new Person();
            personCarl.setName("carl");

            Person personMarry = new Person();
            personMarry.setName("marry");

            List<Person> persons = new ArrayList<>();
            persons.add(personCarl);
            persons.add(personMarry);

            JTree jTree = new JTree(new TreeModel(){
                @Override
                public Object getRoot() {
                    return persons;
                }

                @Override
                public Object getChild(Object parent, int index) {
                    if (parent == persons) {
                        return persons.get(index);
                    }
                    return null;
                }

                @Override
                public int getChildCount(Object parent) {
                    if (parent == persons) {
                        return persons.size();
                    }
                    return 0;
                }

                @Override
                public boolean isLeaf(Object node) {
                    if (node == persons) {
                        return false;
                    }
                    return true;
                }

                @Override
                public void valueForPathChanged(TreePath path, Object newValue) {
                }

                @Override
                public int getIndexOfChild(Object parent, Object child) {
                    if (parent == persons) {
                        return persons.indexOf(child);
                    }
                    return 0;
                }

                @Override
                public void addTreeModelListener(TreeModelListener l) {
                }
                @Override
                public void removeTreeModelListener(TreeModelListener l) {
                }
            });

            jTree.setCellRenderer(new DefaultTreeCellRenderer(){
                @Override
                public Component getTreeCellRendererComponent(JTree tree, Object value, boolean selected, boolean expanded, boolean leaf, int row, boolean hasFocus) {
                    Component component = super.getTreeCellRendererComponent(tree, value, selected, expanded, leaf, row, hasFocus);
                    JLabel jLabel = (JLabel)component;

                    if (value == persons) {
                        jLabel.setText("ROOT");
                    }
                    else {
                        Person person = (Person)value;
                        jLabel.setText(person.getName());
                    }
                    return component;
                }
            });

            JButton button = new JButton("change it");
            button.addActionListener(e -> {
                personCarl.setName(personCarl.getName() + "x");
                System.out.println(personCarl);
            });

            JFrame jFrame = new JFrame();
            jFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            jFrame.getContentPane().setLayout(new FlowLayout());
            jFrame.getContentPane().add(new JScrollPane(jTree));
            jFrame.getContentPane().add(button);
            jFrame.pack();
            jFrame.setVisible(true);
        });
    }
}

The application implements a simple TreeModel, sets up the renderer (you can leave that out if you want - it does not change anything), and once started it displays the tree. If you click the button, the name changes, so the bean's hashcode, and the tree stops painting.

The solution is to have some kind of intermediate "Node" class. But this should not be necessary, the model provides all the information the tree needs. This pattern works perfectly fine on JTable or JList.

Replacing the hashcode with an unchanging one (just swap the lines in the method) makes it all work.

Answer 1

The TreeModel implementation in the question is broken in failing to notify its listeners about changes. After adding the notification - admittedly very coarse in adding api to be used by application code (!don't! - it's model responsibility) that fires a structureChanged for the whole tree - none of the problems is visible:

public static class CustomTreeModel implements TreeModel {
    List<TreeModelListener> listenerList = new ArrayList<>();

    List<Person> persons;

    public CustomTreeModel(List<Person> persons) {
        this.persons = persons;
    }

    public void fireChange(Person person) {
        TreeModelEvent ev = new TreeModelEvent(this, new Object[] {getRoot()});
        listenerList.forEach(l -> l.treeStructureChanged(ev));
    }

    @Override
    public Object getRoot() {
        return persons;
    }

    @Override
    public Object getChild(Object parent, int index) {
        if (parent == persons) {
            return persons.get(index);
        }
        return null;
    }

    @Override
    public int getChildCount(Object parent) {
        if (parent == persons) {
            return persons.size();
        }
        return 0;
    }

    @Override
    public boolean isLeaf(Object node) {
        if (node == persons) {
            return false;
        }
        return true;
    }

    @Override
    public void valueForPathChanged(TreePath path, Object newValue) {
    }

    @Override
    public int getIndexOfChild(Object parent, Object child) {
        if (parent == persons) {
            return persons.indexOf(child);
        }
        // fixed to comply to contract if either parent or child are uncontained 
        return -1;
    }

    @Override
    public void addTreeModelListener(TreeModelListener l) {
        listenerList.add(l);
    }

    @Override
    public void removeTreeModelListener(TreeModelListener l) {
        listenerList.remove(l);
    }


}

and using in the change action:

button.addActionListener(e -> {
    personCarl.setName(personCarl.getName() + "x");
    System.out.println(personCarl);
    treeModel.fireChange(personCarl);
});

Note: this is just for demonstrating the effect of any proper notification, real code should

• let the model do the notification
• use the most limited change type

---

As turned out by combined digging, it's strongly discouraged/disallowed to have nodes that are mutable in equals/hashcode:

Tree and its related classes place TreePaths in Maps. As such if a node is requested twice, the return values must be equal (using the equals method) and have the same hashCode

But even with making both immutable (delegate to super in Person), the model still isn't working as expected - it seems impossible to modify it (f.i. insert a new Person) and make the tree update.

Code snippets to reproduce:

// model api to fire appropriate events 
public void fireInsert(Person person) {
    int index = getIndexOfChild(getRoot(), person);
    TreeModelEvent addEv = new TreeModelEvent(this,
            new Object[] {getRoot()},
            new int[] {index},
            new Object[] {person}
            );
    for(TreeModelListener l: listenerList) {
        l.treeNodesInserted(addEv);
    }
}

// client code to add a new Person    
JButton directAdd = new JButton("direct add");
directAdd.addActionListener(e -> {
    Person person = new Person();
    person.setName(personCarl.getName() + "y");
    persons.add(person);
    treeModel.fireInsert(person);
});

There is no immediate update of the tree, clicking on it later has unpredictable effects (f.i. showing a blank tree, similar as described in the question).

These can be fixed by changing the root to not return the list that describes the content: for some reason (that I don't fully understand, to be honest), having the list object as root confuses the tree.

Fixed model:

public CustomSimpleTreeModel(List<Person> persons) {
    this.persons = persons;
}

String root = "ROOT IN MODEL";
@Override
public Object getRoot() {
    return root;
}

@Override
public int getIndexOfChild(Object parent, Object child) {
    if (parent == getRoot()) {
        return persons.indexOf(child);
    }
    // fixed to comply to contract if either parent or child are uncontained 
    return -1;
}

Aside: this adjustment ameliorates the problems with mutable hash/equals - but even then, we shouldn't :)

Answer 2

As pointed out by Kleopatra (via Twitter) TreeModel requires the nodes to be immutable on hashcode and equals.

https://docs.oracle.com/javase/7/docs/api/index.html?javax/swing/JTree.html

This is caused by the usage of a Hashtable inside a (by name) cache, VariableHeightLayoutCache.treePathMapping. This cache does not really behave like a cache, but like an internal storage. So when the hash changes, the cache lacks data, and that is not handled as a cache miss, but results in painting oddities (as the JavaDoc mentions).

This means using standard domain entities directly as nodes is not possible; these need to be wrapped inside some Node class that meets the immutable requirement.

Side note: the missing listener logic in the example has no influence on this problem.