On Sat, 10 Dec 2022 08:03:27 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> I was just going to write that now that I look at it, this is a list >> comparison according to `List::equals`. No need to redo the whole logic.. >> >> About the validity of the comparison, `ReadOnlyListProperty` also implements >> `List`: >> >> ReadOnlyListProperty<E> extends ListExpression<E> >> ListExpression<E> implements ObservableListValue<E> >> ObservableListValue<E> extends ObservableObjectValue<ObservableList<E>>, >> ObservableList<E> >> ObservableList<E> extends List<E>, Observable >> >> So... it's also a list. > >> So... it's also a list. > > I think it's a big mess, you can already see that in the hierarchy, extending > both `ObservableObjectValue<ObservableList<E>>` (which is > `ObservableValue<T>`) but also being a `ObservableList<E>`. One or the other > is going to break; either it is a property containing a list, or it is a > list, it can't be both. > > Either I can do: > > listProperty.equals(normalList); > > or: > > listProperty.equals(aNormalPropertyContainingAList); > > It looks like the first was chosen (I suppose because the `List` interface > defines `equals` in that way) and the 2nd case will break (and is unfixable > as equals needs to be symmetric). > > Looking a bit further, I think extending > `ObservableObjectValue<ObservableList<E>>` was a mistake here. It's also what > contributes to making the collection properties so confusing (ie. you can > replace all elements, or you can replace the entire list, which are two > different actions that require two different forms of listeners) -- no other > properties allow you to replace it's "container" -- ie. if properties had > used an `AtomicReference` internally to store their value, they would not > offer me the option to replace either the entire `AtomicReference` or only > the value of the reference. I've adjusted it now to be a bit more modern Java as per your suggestion. ------------- PR: https://git.openjdk.org/jfx/pull/969
