ReactFX
ReactFX copied to clipboard
Add class ObservableSortedArraySet.
This PR adds a class ObservableSortedArraySet
, an observable set that is sorted and has an observable list view.
Note that it adds a dependency on Guava, which is used to implement some SortedSet
methods. Not sure if that's acceptable or not.
Thanks for the PR.
My main concern is that the implementation is not safe for recursive changes, i.e. changes made from within a change listener. I realize that observable collections shipped with JavaFX do not support recursive changes either. I find it very unfortunate, though, because it defies local reasoning about the code—when you want to make changes to a collection, you have to be aware whether the code is already executing from within a change listener.
ReactFX's LiveList allows recursive changes.
I would prefer not adding a dependency on Guava, but that's not the biggest issue now (especially when there are only a few uses of it that could be replaced easily).
I've added some tests of recursive changes, and changed ObservableSortedArraySet
to be based on LiveList
. Recursive changes seem to work correctly now. (I tried using FXCollections.observableArrayList
, but you were right—recursive changes did not fire an event like they should have.)
I've also pushed a couple of bug fixes (with their own branches), which my changes depend on. The change with ListIteratorImpl
is needed because LiveList::sort
doesn't work without a mutable ListIterator
.
This leaves the Guava issue. Do you want me to change the subset methods to not depend on it?
Do you want me to change the subset methods to not depend on it?
Adding a dependency to Guava seems an overkill here. Seems to be very easy to replace com.google.common.collect.Sets.filter()
with a pure Java equivalent or maybe some stuff Stream
provides.
Here is a test case that fails. It tests that if a change reports that an element was added, the added element should be in the set.
@Test
public void testRecursiveChanges() {
ObservableSortedArraySet<Integer> set = new ObservableSortedArraySet<>(
Integer::compareTo, i -> Collections.emptyList());
set.addListener((SetChangeListener.Change<?> change) -> {
if(change.wasAdded()) {
set.remove(change.getElementAdded());
}
});
set.addListener((SetChangeListener.Change<?> change) -> {
if(change.wasAdded()) {
// if an element was added, it should be in the set
assertThat(set, contains(change.getElementAdded()));
}
});
set.add(5);
}