On Sat, 10 Dec 2022 08:18:34 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>>> 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. I agree that this is dubious design, and having worked a bit on observable collections I also found some questionable decisions with regards to the type hierarchy. For example. `ListExpression` and some subclasses should have been interfaces, all the way down to `ListPropertyBase` where a state is actually present. As for the equals, I wonder if it should have been defined in `ListExpression` instead, though that would bring the question about bindings - what makes a binding equal to another. Right now, what we're doing right now is copying the `AbstractList` equals implementation essentially. I think it's fine, but points, again, to a possible hierarchy problem. On another note, I'm looking at `ListExpression`'s `add` and other modifying methods. It's calling `EMPTY_LIST.add(element)` where `ObservableList EMPTY_LIST = FXCollections.emptyObservableList();`. Wouldn't that throw an exception since it's an empty unmodifiable observable list? ------------- PR: https://git.openjdk.org/jfx/pull/969
