JTree shows same node as child node

Question

I am using a custom TreeModel for my JTree. I have a root node and only 1 child node that is retrieved by a query from the database. I am able to populate the tree with the desired output.

However, when I click on the child node, it keeps recursively dispalying the same child node and it keeps adding child nodes with the same output. I tried to use static nodes i.e. I created a root node and then added 2 child nodes to it and I observe the same behavior.

My main program

import javax.swing.JFrame;
import javax.swing.JSplitPane;
import javax.swing.SwingUtilities;

public class RunApp {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                ShowFrame f = new ShowFrame();

                f.setSize(600, 600);
                f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                f.setVisible(true);
            }
        });
    }
}

My show_frame class

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.HeadlessException;
import java.util.ArrayList;
import java.util.List;

import javax.swing.JFrame;
import javax.swing.JSplitPane;
import javax.swing.JTabbedPane;
import javax.swing.JTree;
import javax.swing.SwingUtilities;
import javax.swing.tree.DefaultMutableTreeNode;

public class ShowFrame extends JFrame {

    private JSplitPane splitPane;
    private FormPanel formPanel;
    private TreePanel treePanel;
    private JTabbedPane tabPane;
    private List<Objects> instanceDetails= new ArrayList<Objects>();

    public ShowFrame() {
        super("new frame");
        formPanel = new FormPanel();
        instanceDetails.add(new Objects(" "," "," "," "));
        treePanel = new TreePanel(instanceDetails);
        tabPane = new JTabbedPane();
        tabPane.add(treePanel);

        splitPane = new JSplitPane(JSplitPane.HORIZONTAL_SPLIT, formPanel,
                tabPane);
        splitPane.setOneTouchExpandable(true);

        setMinimumSize(new Dimension(500, 500));
        add(splitPane, BorderLayout.CENTER);
    }
}

This is where I create my TreePanel

import java.util.List;

import javax.swing.JPanel;
import javax.swing.JTree;
import javax.swing.event.TreeSelectionEvent;
import javax.swing.event.TreeSelectionListener;
import javax.swing.tree.DefaultMutableTreeNode;
import javax.swing.tree.TreePath;
import javax.swing.tree.TreeSelectionModel;

public class TreePanel extends JPanel {

    private int count = 0;

    private JTree tree;
    private List<Objects> instanceDetails;
    private MyTreeModel gm;
    private DefaultMutableTreeNode root = new DefaultMutableTreeNode();

    private Controller c = new Controller();

    public TreePanel(List<Objects> instanceDetails) {
        this.instanceDetails = instanceDetails;
        tree = new JTree();

        if (instanceDetails.get(0).getObjectId() == " ") {
            tree.setModel(new MyTreeModel(root));

        } else {
            tree.setModel(new MyTreeModel(treeNodes(instanceDetails)));
        }

        gm = new MyTreeModel(root);
        gm.fireTreeStructureChanged(root);

        tree.getSelectionModel().setSelectionMode(
                TreeSelectionModel.SINGLE_TREE_SELECTION);
        add(tree);


    }

    private DefaultMutableTreeNode treeNodes(List<Objects> instanceDetails) {
        for (Objects id : instanceDetails) {
            count++;

            DefaultMutableTreeNode objs = new DefaultMutableTreeNode(count + " : " + id.getType()
                    + " : " + id.getObjectId() + " : " + id.getStatus() + " : "
                    + id.getCondition());

            root.add(objs);
        }

        return root;
    }

}

My tree model

import java.util.Vector;

import javax.swing.event.TreeModelEvent;
import javax.swing.event.TreeModelListener;
import javax.swing.tree.DefaultMutableTreeNode;
import javax.swing.tree.TreeModel;
import javax.swing.tree.TreeNode;
import javax.swing.tree.TreePath;


public class MyTreeModel implements TreeModel {

    public static Vector<TreeModelListener> treeModelListeners =
        new Vector<TreeModelListener>();

    private static DefaultMutableTreeNode rootPerson;

     public MyTreeModel(DefaultMutableTreeNode nodes) {
        rootPerson = nodes;
    }

     //////////////// Fire events //////////////////////////////////////////////

    /**
     * The only event raised by this model is TreeStructureChanged with the
     * root as path, i.e. the whole tree has changed.
     */
    protected void fireTreeStructureChanged(DefaultMutableTreeNode rootPerson) {
        TreeModelEvent e = new TreeModelEvent(this, new Object[] {rootPerson});
        for (TreeModelListener tml : treeModelListeners) {
            tml.treeStructureChanged(e);
        }
    }


     //////////////// TreeModel interface implementation ///////////////////////

    /**
     * Adds a listener for the TreeModelEvent posted after the tree changes.
     */
    public void addTreeModelListener(TreeModelListener l) {
        treeModelListeners.addElement(l);    
       }

    /**
     * Returns the child of parent at index index in the parent's child array.
     */
    public Object getChild(Object parent, int index) {
        return rootPerson.getChildAt(index);
    }

    /**
     * Returns the number of children of parent.
     */
    public int getChildCount(Object parent) {
        return 1;
        //rootPerson.getLeafCount()

    }

    /**
     * Returns the index of child in parent.
     */
    public int getIndexOfChild(Object parent, Object child) {
        return rootPerson.getIndex((DefaultMutableTreeNode) child);
    }

    /**
     * Returns the root of the tree.
     */
    public Object getRoot() {
        return rootPerson;
    }

    /**
     * Returns true if node is a leaf.
     */
    public boolean isLeaf(Object node) {
        return rootPerson.isLeaf();
    }

    /**
     * Removes a listener previously added with addTreeModelListener().
     */
    public void removeTreeModelListener(TreeModelListener l) {
       //removeTreeModelListener(l);
    }

    /**
     * Messaged when the user has altered the value for the item
     * identified by path to newValue.  Not used by this model.
     */
    public void valueForPathChanged(TreePath path, Object newValue) {
    }

}

and remove the comments in the self explanatory methods names — Francisco Puga
just a couple of comments: a) having an instance variable (aka: static field) for all listeners is evil, it's the exact same list for instances b) there are several models around (one in tree, one separately) c) never-ever fire any events on behalf of the model, that's its own responsibility d) no need for a custom model if you use DefaultMutableTreeNodes (that's what DefaultTreeModel is meant for) e) if you insist on rolling your own: never-ever implement the add/removeListener to do nothing - nearly all misbehaviour is related to incorrect notification. Out-of-space - so good luck :-) — kleopatra

Guillaume Polet Guillaume Polet · Accepted Answer · 2013-02-26T11:25:21

Your implementation of TreeModel is clumsy and is the cause of your issues:

public static Vector<TreeModelListener> treeModelListeners =
    new Vector<TreeModelListener>();

private static DefaultMutableTreeNode rootPerson;

--> Bad, Bad, Bad, ... real bad. There is absolutely no need to make these statements static and this will cause severe issues if you happen to create 2 different instances

/**
 * Returns the child of parent at index index in the parent's child array.
 */
public Object getChild(Object parent, int index) {
    return rootPerson.getChildAt(index);
}

Here, no matter which parent is provided, you return always the same child (hence this is why you see the same child over and over). The code should be return (parent==rootPerson?rootPerson.getChildAt(index):null);

/**
 * Returns the number of children of parent.
 */
public int getChildCount(Object parent) {
    return 1;
    //rootPerson.getLeafCount()

}

Same as previous comment, you don't look what is the parent. Code should be return (parent==rootPerson?1:0);

/**
 * Returns the index of child in parent.
 */
public int getIndexOfChild(Object parent, Object child) {
    return rootPerson.getIndex((DefaultMutableTreeNode) child);
}

Same as previous comment, you don't look what is the parent. Code should be return (parent==rootPerson?rootPerson.getIndex((DefaultMutableTreeNode) child):-1);

/**
 * Returns true if node is a leaf.
 */
public boolean isLeaf(Object node) {
    return rootPerson.isLeaf();
}

Again, same mistake, you don't care about node

/**
 * Removes a listener previously added with addTreeModelListener().
 */
public void removeTreeModelListener(TreeModelListener l) {
   //removeTreeModelListener(l);
}

Why don't you implement properly removeTreeModelListener? (and as suggested by @trashgod, you can always use the default EventListenerList which does most of the work for you)

Conclusion: your implementation of TreeModel is full of bugs and this is why you get the problem you describe. Now, since you are using DefaultMutableTreeNode, I can only encourage you to also use DefaultTreeModel which will handle everything for you and avoid you to have to re-implement this, with all the "risks" it implies.

JTree shows same node as child node

1 Answers