Why is the ChangeListener associated to a CheckBox running multiple times in the TreeCell

Question

I am having a problem that I cannot figure out. I am taking a TreeView called treeModel and setting cells using setCellFactory as can be seen by the code. Now within the updateItem, I am setting a CheckBox as a graphic and am associating it with the CheckBoxTreeItem custom class called CheckBoxTreeItemModel. Now every time updateItem runs a new CheckBox is created and a new ChangeListener is created for it.

Now at first everything looks normal. Then I expand the direct children of the root, and begin checking item, but the listener seems to be called multiple time. For every level of TreeItems that is expanded, that is how many times the listener is called on one of the descendants of root. If I click on a child a few leaves down a parent, those listeners are then called multiple times as well. Its weird behavior that might be hard to explain, but the point is I don't think the listener is suppose to be called that many times. Its as if its cached. The problem code is below. Any help understanding why this may be happening would be greatly appreciated.

    treeModel.setCellFactory(new Callback<TreeView<String>, TreeCell<String>>() {

        @Override
        public TreeCell<String> call(TreeView<String> param) {
            return new TreeCell<String>() {
                @Override
                public void updateItem(String item, boolean empty) {
                    super.updateItem(item, empty);
                    final TreeCell<String> currCell = this;
                    this.setOnMouseClicked(new EventHandler<MouseEvent>() {
                        /*mouse event stuff completely unrelated to problem*/
                    });
                    if (empty) {
                        setText(null);
                        setGraphic(null);
                    } 
                    else {

                        TreeItem<String> treeItem = getTreeItem();
                        if (treeItem instanceof CheckBoxTreeItemModel) {
                            System.out.println("Being called.");
                            final CheckBoxTreeItemModel chkTreeItem = (CheckBoxTreeItemModel) treeItem;

                            setText(item.toString());
                            CheckBox chk = new CheckBox();
                            chk.setSelected(chkTreeItem.getDeleteTick());

                            if(chkTreeItem.getListener() == null) {
                                ChangeListener<Boolean> listener = new ChangeListener<Boolean>() {
                                    @Override
                                    public void changed(ObservableValue<? extends Boolean> observable,
                                            Boolean oldValue, Boolean newValue) {
                                        if(newValue) {
                                            //was checked
                                            System.out.println(chkTreeItem.toString()+" was checked!");
                                            chkTreeItem.setDeleteTick(newValue);    
                                        }
                                        else {
                                            System.out.println(chkTreeItem.toString()+" was un-checked!");
                                            chkTreeItem.setDeleteTick(newValue);    
                                        }

                                    }//end of changed method
                                };
                                chkTreeItem.setListener(listener);
                            }
                            chk.selectedProperty().removeListener(chkTreeItem.getListener());
                            chk.selectedProperty().addListener(chkTreeItem.getListener());

                            chk.indeterminateProperty().bindBidirectional(chkTreeItem.indeterminateProperty());
                            chk.selectedProperty().bindBidirectional(chkTreeItem.selectedProperty());
                            setGraphic(chk);
                        } 
                        else {
                            setText(item.toString());
                            setGraphic(null);
                        }
                    }
                }//end of updateItem
            };
        }//end of the call method
    });

Answer 1

Suggested Approach

I advise scrapping most of your code and using inbuilt CheckBoxTreeCell and CheckBoxTreeItem classes instead. I'm not sure if the inbuilt cells match your requirements, but even if they don't, you could examine the source code for them and compare it with yours and it (should) start to give you a good idea on where you are going wrong.

Potential Issues in Your Code

Reproducing your issue would require more code than you currently supply. But some things to look for are:

1. Removing and adding the same listener is pointless:

   // first line is redundant. 
   // all listener code is probably unnecessary as you already bindBidirectional.
   chk.selectedProperty().removeListener(chkTreeItem.getListener());
   chk.selectedProperty().addListener(chkTreeItem.getListener());
   

2. updateItem might be called many times for a given cell. don't create new nodes for the cell everytime, instead re-use existing nodes created for the cell.

   // replace with a lazily instantiated CheckBox member reference in TreeCell instance.
   CheckBox chk = new CheckBox();  
   

3. You bindBiDirectional but never unbind those bound values.

   // should also unbind this values.
   chk.indeterminateProperty().bindBidirectional(chkTreeItem.indeterminateProperty());
   chk.selectedProperty().bindBidirectional(chkTreeItem.selectedProperty());
   

Sample Code

Sample updateItem code from the inbuilt CheckBoxTreeCell code:

public class CheckBoxTreeCell<T> extends DefaultTreeCell<T> {
. . .
private final CheckBox checkBox;    
private ObservableValue<Boolean> booleanProperty;    
private BooleanProperty indeterminateProperty;
. . .
@Override public void updateItem(T item, boolean empty) {
    super.updateItem(item, empty);

    if (empty) {
        setText(null);
        setGraphic(null);
    } else {
        StringConverter c = getConverter();

        TreeItem<T> treeItem = getTreeItem();

        // update the node content
        setText(c != null ? c.toString(treeItem) : (treeItem == null ? "" : treeItem.toString()));
        checkBox.setGraphic(treeItem == null ? null : treeItem.getGraphic());
        setGraphic(checkBox);

        // uninstall bindings
        if (booleanProperty != null) {
            checkBox.selectedProperty().unbindBidirectional((BooleanProperty)booleanProperty);
        }
        if (indeterminateProperty != null) {
            checkBox.indeterminateProperty().unbindBidirectional(indeterminateProperty);
        }

        // install new bindings.
        // We special case things when the TreeItem is a CheckBoxTreeItem
        if (treeItem instanceof CheckBoxTreeItem) {
            CheckBoxTreeItem<T> cbti = (CheckBoxTreeItem<T>) treeItem;
            booleanProperty = cbti.selectedProperty();
            checkBox.selectedProperty().bindBidirectional((BooleanProperty)booleanProperty);

            indeterminateProperty = cbti.indeterminateProperty();
            checkBox.indeterminateProperty().bindBidirectional(indeterminateProperty);
        } else {
            Callback<TreeItem<T>, ObservableValue<Boolean>> callback = getSelectedStateCallback();
            if (callback == null) {
                throw new NullPointerException(
                        "The CheckBoxTreeCell selectedStateCallbackProperty can not be null");
            }

            booleanProperty = callback.call(treeItem);
            if (booleanProperty != null) {
                checkBox.selectedProperty().bindBidirectional((BooleanProperty)booleanProperty);
            }
        }
    }
}
. . .
}