JavaFX ListChangeListener обрабатывает 'removeAll (Collection)' непоследовательно, в зависимости от положения удаленных элементов в ObservableList - PullRequest
0 голосов
/ 02 апреля 2020

Я столкнулся с аномалией в том, как ListChangeListener обрабатывает пакетное удаление (т. Е. removeAll(Collection). Если элементы в Collection являются непрерывными , тогда операция обработки, указанная в слушатель работает нормально. Однако, если Collection являются не смежными, то операция, указанная в слушателе, останавливается после смежности .

Это лучше всего объяснить Например, ObservableList состоит из следующих элементов:

  • «красный»
  • «оранжевый»
  • «желтый»
  • "зеленый"
  • "синий"

Предположим также, что есть отдельный ObservableList, который отслеживает значения hashCode для цветов и что ListChangeListener было добавлено, что удаляет hashCode из второго списка всякий раз, когда удаляется один или несколько элементов в первом списке.Если «удаление» Collection состоит из «красный», «оранжевый» и «желтый», то код в слушателе удаляет hashCodes для всех трех элементов из второго списка, как и ожидалось. Однако, если «remove» Collection состоит из «red», «orange» и « green », то код в слушателе останавливается после удаления hashCode для «orange» и никогда не достигает "зеленый", как и должно быть.

Краткое приложение, иллюстрирующее проблему, изложено ниже. Код слушателя находится в методе с именем buildListChangeListener(), который возвращает слушателя, добавленного в список «Цвета». Для запуска приложения полезно знать, что:

  • 'последовательный' в ComboBox указывает три цвета, которые смежные , как объяснено выше; нажатие кнопки «Удалить» приведет к их удалению из списка «Цвета», а их hashCodes из другого списка.
  • «сломанный» указывает три цвета, которые не являются смежными таким образом, нажатие кнопки «Удалить» удаляет только один из цветов
  • нажатие «Refre sh» восстанавливает оба списка в исходное состояние

Вот код приложения :

    package test;

import static java.util.Objects.isNull;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import javafx.application.Application;
import javafx.application.Platform;
import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;
import javafx.event.EventHandler;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.canvas.Canvas;
import javafx.scene.canvas.GraphicsContext;
import javafx.scene.control.Button;
import javafx.scene.control.ComboBox;
import javafx.scene.control.ContentDisplay;
import javafx.scene.control.Label;
import javafx.scene.control.ListCell;
import javafx.scene.control.ListView;
import javafx.scene.layout.Background;
import javafx.scene.layout.BackgroundFill;
import javafx.scene.layout.CornerRadii;
import javafx.scene.layout.HBox;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.stage.Stage;
import javafx.stage.WindowEvent;
import javafx.util.Pair;

public class RemoveAllItemsBug extends Application {

     private StackPane stackPane;
     private HBox hbox;

     private VBox vbox1;
     private Label label1;
     private ListView<Pair<String, Color>> colors;

     private VBox vbox2;
     private Label label2;
     private ListView<Integer> hashCodes;

     private VBox vbox3;
     private Label label3;
     private ComboBox<String> actionModes;
     private Button btnRemove;
     private Button btnRefresh;

     final static private String CONSECUTIVE = "consecutive", BROKEN = "broken";

     private final EventHandler<WindowEvent> onCloseRequestListener = evt -> {
          Platform.exit();
          System.exit(0);
     };

     @Override
     public void start(Stage primaryStage) throws Exception {
          primaryStage.setTitle("DUMMY  APP");

          // Necessary to ensure stage closes completely and javaw.exe stops running
          primaryStage.setOnCloseRequest(onCloseRequestListener);

          primaryStage.setWidth(550);
          primaryStage.setHeight(310);

          //          primaryStage.setMinWidth(550);
          //          primaryStage.setMinHeight(310);

          /*
           * Code block below for width/height property printouts is used to
           * test for an optimal size for the app. Once the size is determined
           * they may (and should be) commented out as here.
           */
          primaryStage
                      .widthProperty()
                      .addListener((width, oldWidth, newWidth) -> {
                           System.out.println("width: " + newWidth);
                      });

          primaryStage
                      .heightProperty()
                      .addListener((height, oldHeight, newHeight) -> {
                           System.out.println("height: " + newHeight);
                      });

          initializeUI();
          installSimpleBehavior();
          installListChangeListener();

          primaryStage.setScene(new Scene(stackPane));
          primaryStage.show();
     }

     private void installListChangeListener() {
          /*
           * The 'listChangeListenerUsingIf()' method returns a listener that
           * uses an 'if (c.next()) ...' statement to access the first change in
           * the Change variable (c). For purposes of accessing the first change
           * this is functionally equivalent to a 'while (c.next()) ...'
           * statement. However, because the Change variable may contain
           * multiple 'remove' changes where each change is represented by a
           * separate 'getRemoved()' list, the 'if (c.next())' statement will
           * catch only the first change while the 'while (c.next())' statement
           * (which is used in the 'listChangeListenerUsingWhile()' method)
           * catches them all.
           * 
           * The code below should be commented out as appropriate before
           * running the app in order to see the difference.
           * 
           * This case illustrates a serious flaw in the ListChangeListener API
           * documentation because it fails to indicate that the Change variable
           * may include multiple 'remove' changes and that each such change
           * must be accessed in a separate iteration (e.g. the 'while
           * (c.next()...').
           * 
           * In contrast, 'add' changes (i.e. changes resulting from the
           * addition of one or more items to the source list), the name of the
           * method that returns the change(s) is 'getAddSublist()'. This
           * clearly indicates that there may be more than one list of items
           * that have been added, or similarly that the total items that have
           * been 'added' by the change(s) represented by the Change variable
           * may be included in more than one list; thus the use of the term
           * 'sublist'.
           * 
           * The flaw is illustrated further in the cautionary note in the API
           * that reads as follows:
           * 
           * "[I]n case the change contains multiple changes of different type,
           * these changes must be in the following order: <em> permutation
           * change(s), add or remove changes, update changes </em> This is
           * because permutation changes cannot go after add/remove changes as
           * they would change the position of added elements. And on the other
           * hand, update changes must go after add/remove changes because they
           * refer with their indexes to the current state of the list, which
           * means with all add/remove changes applied."
           * 
           * This is certainly useful information. However, the problems
           * illustrated by the case at hand (i.e. different treatment based on
           * whether the changed items are continguous in the source list) are
           * just as significant as the situation addressed by the note, yet
           * they are not mentioned.
           * 
           * A better understanding as to how the process works can be gained by
           * running a system printout for the Change variable class
           * (System.out.println("Change variable class: " +
           * c.getClass().getSimpleName())) and compare the results yielded from
           * changing the choice in the 'Action modes' combo box from
           * 'consecutive' to 'broken'. For 'consecutive' (i.e. continguous),
           * the class for the Change variable is
           * ListChangeBuilder$SingleChange, for 'broken' (i.e. non-continguous)
           * the class is ListChangeBuilder$IterableChange. These classes aren't
           * well documented, which while regrettable is understandable inasmuch
           * as they're private inner classes for restricted API. Interestingly,
           * however, there is a public class MultipleAdditionAndRemovedChange
           * (also restricted API) that appears to fit this case perfectly and
           * is a bit more informative.
           */

          //          colors.getItems().addListener(listChangeListenerUsingIf());
          colors.getItems().addListener(listChangeListenerUsingWhile());
     }

     private void initializeUI() {

          //- Controls for colors
          label1 = new Label("Colors");
          colors = new ListView<Pair<String, Color>>();
          colors.setPrefSize(150, 200);
          colors.setItems(FXCollections.observableList(new ArrayList<>(colorsList())));
          vbox1 = new VBox(label1, colors);

          //- Controls for colors
          label2 = new Label("Hash codes");
          hashCodes = new ListView<Integer>();
          hashCodes.setPrefSize(150, 200);
          hashCodes.setItems(FXCollections.observableList(new ArrayList<>(
                                                                          colorsList().stream()
                                                                                      .map(e -> e.hashCode())
                                                                                      .collect(Collectors.toCollection(ArrayList::new)))));
          vbox2 = new VBox(label2, hashCodes);

          //- 'Action mode' controls
          label3 = new Label("Action mode");
          actionModes = new ComboBox<>(
                                       FXCollections.observableList(List.of(CONSECUTIVE, BROKEN)));
          actionModes.setPrefWidth(150);
          actionModes.getSelectionModel().select(0);
          btnRemove = new Button("Remove");
          btnRefresh = new Button("Refresh");
          List.of(btnRemove, btnRefresh).forEach(b -> {
               b.setMaxWidth(Double.MAX_VALUE);
               VBox.setMargin(b, new Insets(5, 0, 0, 0));
          });
          vbox3 = new VBox(label3, actionModes, btnRemove, btnRefresh);

          hbox = new HBox(vbox1, vbox2, vbox3);
          hbox.setPadding(new Insets(10));
          hbox.setSpacing(15);
          hbox.setBackground(new Background(
                                            new BackgroundFill(Color.DARKGRAY, CornerRadii.EMPTY, Insets.EMPTY),
                                            new BackgroundFill(Color.WHITESMOKE, CornerRadii.EMPTY, new Insets(1))));

          stackPane = new StackPane(hbox);
          stackPane.setPadding(new Insets(15));
     }

     private void installSimpleBehavior() {

          //- 'Colors' cell factory
          colors.setCellFactory(listView -> {
               return new ListCell<Pair<String, Color>>() {

                    @Override
                    protected void updateItem(Pair<String, Color> item, boolean empty) {
                         super.updateItem(item, empty);
                         if (isNull(item) || empty) {
                              setGraphic(null);
                              setText(null);
                         }
                         else {
                              HBox graphic = new HBox();
                              graphic.setPrefSize(15, 15);
                              graphic.setBackground(new Background(new BackgroundFill(
                                                                                      item.getValue(),
                                                                                      CornerRadii.EMPTY,
                                                                                      Insets.EMPTY)));
                              setGraphic(graphic);
                              setText(item.getKey());
                              setContentDisplay(ContentDisplay.LEFT);
                         }
                    }
               };
          });

          //- 'Colors' cell factory
          hashCodes.setCellFactory(listView -> {
               return new ListCell<Integer>() {

                    @Override
                    protected void updateItem(Integer item, boolean empty) {
                         super.updateItem(item, empty);
                         if (isNull(item) || empty) {
                              setGraphic(null);
                              setText(null);
                         }
                         else {
                              HBox graphic = new HBox();
                              graphic.setPrefSize(15, 15);
                              graphic.setBackground(new Background(new BackgroundFill(
                                                                                      colorForHashCode(item),
                                                                                      CornerRadii.EMPTY,
                                                                                      Insets.EMPTY)));
                              Canvas c = new Canvas(15, 15);
                              GraphicsContext graphics = c.getGraphicsContext2D();
                              graphics.setFill(colorForHashCode(item));
                              graphics.fillRect(0, 0, c.getWidth(), c.getHeight());
                              setGraphic(c);
                              setText("" + item);
                              setContentDisplay(ContentDisplay.LEFT);
                         }
                    }

                    private Color colorForHashCode(int hash) {
                         return colorsList().stream()
                                            .filter(e -> e.hashCode() == hash)
                                            .map(e -> e.getValue())
                                            .findFirst()
                                            .orElseThrow();
                    }
               };
          });

          //- 'Remove' button action
          btnRemove.setOnAction(e -> {
               String actionMode = actionModes.getValue();
               if (CONSECUTIVE.equals(actionMode)) {
                    colors.getItems().removeAll(consecutiveColors());
               }
               else if (BROKEN.equals(actionMode)) {
                    colors.getItems().removeAll(brokenColors());
               }
          });

          //- 'Refresh' button action
          btnRefresh.setOnAction(e -> {
               colors.getItems().setAll(colorsList());
               hashCodes.getItems().setAll(colorsList()
                                                       .stream()
                                                       .map(ee -> ee.hashCode())
                                                       .collect(Collectors.toCollection(ArrayList::new)));
          });
     }

     private ListChangeListener<Pair<String, Color>> listChangeListenerUsingIf() {
          return c -> {
               if (c.next()) {
                    System.out.println("Change variable class: " + c.getClass().getName());
                    if (c.wasRemoved()) {
                         System.out.println("Removing " + c.getRemovedSize() + " items");
                         c.getRemoved().forEach(e -> {
                              Integer hash = Integer.valueOf(e.hashCode());
                              hashCodes.getItems().remove(hash);
                         });
                         System.out.println("number of 'hash codes' after removal: " + hashCodes.getItems().size());
                         System.out.println();
                    }
                    if (c.wasAdded()) {
                         c.getAddedSubList().forEach(e -> {
                              if (hashCodes.getItems().stream().noneMatch(ee -> ee == e.hashCode()))
                                   hashCodes.getItems().add(e.hashCode());
                         });
                    }
               }
          };
     }

     private ListChangeListener<Pair<String, Color>> listChangeListenerUsingWhile() {
          return c -> {
               while (c.next()) {
                    System.out.println("Change variable class: " + c.getClass().getName());
                    if (c.wasRemoved()) {
                         System.out.println("Removing " + c.getRemovedSize() + " items");
                         c.getRemoved().forEach(e -> {
                              Integer hash = Integer.valueOf(e.hashCode());
                              hashCodes.getItems().remove(hash);
                         });
                         System.out.println("number of 'hash codes' after removal: " + hashCodes.getItems().size());
                         System.out.println();
                    }
                    if (c.wasAdded()) {
                         c.getAddedSubList().forEach(e -> {
                              if (hashCodes.getItems().stream().noneMatch(ee -> ee == e.hashCode()))
                                   hashCodes.getItems().add(e.hashCode());
                         });
                    }
               }
          };
     }

     private List<Pair<String, Color>> colorsList() {
          return List.of(
                         new Pair<>("rot", Color.RED),
                         new Pair<>("orange", Color.ORANGE),
                         new Pair<>("gelb", Color.YELLOW),
                         new Pair<>("grün", Color.GREEN),
                         new Pair<>("blau", Color.BLUE),
                         new Pair<>("violett", Color.PURPLE),
                         new Pair<>("grau", Color.GRAY),
                         new Pair<>("schwarz", Color.BLACK));
     }

     private List<Pair<String, Color>> consecutiveColors() {
          return List.of(
                         new Pair<>("gelb", Color.YELLOW),
                         new Pair<>("grün", Color.GREEN),
                         new Pair<>("blau", Color.BLUE));
     }

     private List<Pair<String, Color>> brokenColors() {
          return List.of(
                         new Pair<>("rot", Color.RED),
                         new Pair<>("grün", Color.GREEN),
                         new Pair<>("blau", Color.BLUE));
     }

     public static void main(String[] args) {
          launch(args);
     }
}

Заранее благодарен за любые отзывы.

[Редактировать с учетом первого комментария @ Slaw]

В этом случае возникает несколько проблем. Первый комментарий @ Slaw заставил меня взглянуть на это иначе. @Slaw прав в том, что использование предложения while (c.next()) ... устраняет проблему, которая возникает при использовании предложения if (c.next())....

Однако, если рассматривать это целостным образом, возникает более фундаментальная проблема: не так много, чтобы использовать предложение if (c.next()), но замаскировал эту ошибку и сделал ее очень трудной для обнаружения. Эта проблема - ужасная документация для класса ListChangeListener.

Я изменил код для примера приложения, чтобы включить второй метод прослушивателя, который работает должным образом (с изменением имени на тот, который вызвал ошибку ), вместе с комментарием относительно того, почему это было необходимо и как ListChangeListener и, в частности, его Change компаньон, похоже, работают. Соответствующие части этого комментария повторяются ниже:

Метод listChangeListenerUsingIf() возвращает слушателя, который использует оператор if (c.next()) ... для доступа к первому изменению в переменной Change (c). В целях доступа к первому изменению это функционально эквивалентно выражению while (c.next()) .... Однако, поскольку переменная Change может содержать несколько «удаленных» изменений, где каждое изменение представлено отдельным списком getRemoved(), оператор if (c.next()) будет перехватывать только первое изменение, тогда как оператор while (c.next()) (который используется в метод listChangeListenerUsingWhile()) перехватывает их все.

Этот случай иллюстрирует серьезный недостаток в документации ListChangeListener API, поскольку в нем не указано, что переменная Change может содержать несколько изменений 'remove' и что каждый такое изменение должно быть доступно в отдельной итерации (например, while (c.next()...).

В отличие от этого, для добавления изменений (т. е. изменений, возникающих в результате добавления одного или нескольких элементов в список источников), имя метода, который возвращает изменение (я): getAddedSublist(). Это ясно указывает на то, что может быть добавлено более одного списка элементов или аналогичным образом, что общее количество элементов, которые были «добавлены» изменениями, представленными переменной Change, может быть включено в более чем один список; таким образом, использование термина sublist.

Недостаток проиллюстрирован далее в предостерегающей записке в API, которая гласит:

"[I] В случае, если изменение содержит несколько изменений различного типа, эти изменения должны быть в следующем порядке: изменение (изменения) перестановки, добавлять или удалять изменения, обновлять изменения Это происходит потому, что изменения перестановки не могут go после добавления / удаления изменений, поскольку они будут меняться положение добавленных элементов. С другой стороны, изменения должны обновляться go после добавления / удаления изменений, поскольку они ссылаются своими индексами на текущее состояние списка, что означает, что все изменения добавлены / удалены применены. "

Это, безусловно, полезная информация. Тем не менее, проблемы, проиллюстрированные рассматриваемым случаем (т. Е. Другой подход, основанный на том, являются ли измененные элементы случайными в списке источников), так же важны, как и ситуация, описанная в примечании; пока они не упомянуты.

Лучшего понимания того, как работает процесс, можно получить, запустив системную распечатку для класса переменных Change (System.out.println("Change variable class: " + c.getClass().getSimpleName())) и сравнив результаты, полученные при изменении выбора. в поле со списком «Режимы действия» от «последовательный» до «сломанный». Для «последовательного» (то есть непрерывного ) класс для переменной Change равен ListChangeBuilder$SingleChange, для «сломанного» (т. Е. не непрерывного ) класс равен ListChangeBuilder$IterableChange , Эти классы недостаточно хорошо документированы, что, к сожалению, понятно, поскольку они являются закрытыми внутренними классами для ограниченного API. Интересно, однако, что существует публичный c класс MultipleAdditionAndRemovedChange (также с ограниченным API), который идеально подходит для этого случая и немного более информативен.

Надеюсь, это поможет, и благодаря @Slaw за полезный ввод.

1 Ответ

2 голосов
/ 03 апреля 2020

Из документации ListChangeListener.Change:

Представляет отчет об изменениях, внесенных в ObservableList. Изменение может состоять из одного или нескольких фактических изменений и должно повторяться путем вызова метода next() [выделение добавлено] .

В вашей реализации из ListChangeListener у вас есть:

if (c.next()) {
  // handle change...
}

Это будет обрабатывать только одно изменение. Вам нужно l oop отменить (то есть повторить) изменения, если их несколько:

while (c.next()) {
 // handle change...
}

Просто изменить это значение if на while в вашем примере устраняет проблему, которую вы описываете.

Вот пример, показывающий, как массовое удаление несмежных элементов приводит к множественным изменениям, объединенным в один объект ListChangeListener.Change:

import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;

public class Main {

  public static void main(String[] args) {
    var list = FXCollections.observableArrayList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
    list.addListener(
        (ListChangeListener<Integer>)
            c -> {
              System.out.println("----------BEGIN_CHANGE----------");
              while (c.next()) {
                // for example, assume c.wasRemoved() returns true
                System.out.printf(
                    "Removed %d element(s): %s%n", c.getRemovedSize(), c.getRemoved());
              }
              System.out.println("-----------END_CHANGE-----------");
            });
    list.removeAll(1, 7, 3, 8, 2, 10);
  }
}

И вывод :

----------BEGIN_CHANGE----------
Removed 3 element(s): [1, 2, 3]
Removed 2 element(s): [7, 8]
Removed 1 element(s): [10]
-----------END_CHANGE-----------

Если вы знакомы с JDB C, вы заметите, что API для итерации ListChangeListener.Change аналогичен итерации ResultSet.

...