Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8092821

[TableView] TableView et al: support update for items that are equal

    Details

      Description

      This is related to RT-22463: TableView et al are not updated on setting items which are equal but have different properties (happens f.i. contexts using POJO from backend).

          ObservableList<Person> persons1 = createPersons1();
          ObservableList<Person> persons2 = createPersons2();
          // such that
          persons1.equals(persons2);
          foreach i: // that is the name property is not part of its equality condition
              !persons1.get(i).getName() .equals(persons2.get(i).getName();

      To reproduce,
      - take the example provided by the reporter,
      - comment the items.clear in the eventHandler (that was the original bug, fixed now)
      - run and press refresh button
      - expected: Name cell content changed from "name" to "updated"
      - actual: Name cell content unchanged

      The underlying reason why no update happens is that the items are checked for equality on cell-level, f.i. in TableRow (similar in all cell implementations):

                  final T newValue = items.get(newIndex);

                  // RT-35864 - if the index didn't change, then avoid calling updateItem
                  // unless the item has changed.
                  if (oldIndex == newIndex) {
                 ---------> here's the equality check: that prevents updating the item
                      if (oldValue != null ? oldValue.equals(newValue) : newValue == null) {
                          break outer;
                      }
                  }
                  updateItem(newValue, false);

      There are good reasons to not call updateItem "unless the item has changed" - hard-coding that "changed" as equality introduces this issue.

      There is no easy/clean way for client code to solve because updateItem(int) is private. All it can do is to wrap super's call of updateIndex(int) and jump in if the new item is equal but not the same, something like

          @Override
          public void updateIndex(int i) {
              int oldIndex = getIndex();
              T oldItem = getItem();
              boolean wasEmpty = isEmpty();
              super.updateIndex(i);
              updateItemIfNeeded(oldIndex, oldItem, wasEmpty);
          }

          /**
           * Here we try to guess whether super updateIndex didn't update the item if
           * it is equal to the old.
           *
           * Strictly speaking, an implementation detail.
           *
           * @param oldIndex cell's index before update
           * @param oldItem cell's item before update
           * @param wasEmpty cell's empty before update
           */
          protected void updateItemIfNeeded(int oldIndex, T oldItem, boolean wasEmpty) {
              // weed out the obvious
              if (oldIndex != getIndex()) return;
              if (oldItem == null || getItem() == null) return;
              if (wasEmpty != isEmpty()) return;
              // here both old and new != null, check whether the item had changed
              if (oldItem != getItem()) return;
              // unchanged, check if it should have been changed
              T listItem = getTableView().getItems().get(getIndex());
              // update if not same
              if (oldItem != listItem) {
                  // doesn't help much because itemProperty doesn't fire
                  // so we need the help of the skin: it must listen
                  // to invalidation and force an update if
                  // its super wouldn't get a changeEvent
                  updateItem(listItem, isEmpty());
              }
          }

      (see IdentityCheckingXX in https://github.com/kleopatra/swingempire-fx/tree/master/fx8-swingempire/src/java/de/swingempire/fx/scene/control/cell )

      My proposal: in updateItem(int) soft-code the decision of what constitutes a change into protected api that can be overridden by subclasses:

                  final T newValue = items.get(newIndex);

                  // RT-35864 - if the index didn't change, then avoid calling updateItem
                  // unless the item has changed.
                  if (oldIndex == newIndex) {
                 ---------> here replace with soft-coded
                      if (!isChanged(oldValue, newValue) {
                          break outer;
                      }
                  }
                  updateItem(newValue, false);
              
              // default implementation tests against equality
              protected boolean isChanged(T oldValue, T newValue) {
                    return oldValue != null ? oldValue.equals(newValue) : newValue == null;
              }
                 

        Attachments

          Activity

            People

            • Assignee:
              jgiles Jonathan Giles
              Reporter:
              fastegal Jeanette Winzenburg
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Imported: