On Wed, 28 Dec 2022 18:57:23 GMT, Nir Lisker <[email protected]> wrote:
>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> modules/javafx.base/src/main/java/com/sun/javafx/binding/BidirectionalContentBinding.java
> line 81:
>
>> 79: if ((obj1 instanceof ObservableList<?> list1) && (obj2
>> instanceof ObservableList<?> list2)) {
>> 80: @SuppressWarnings("unchecked")
>> 81: final ListContentBinding<Object> binding = new
>> ListContentBinding<>((ObservableList<Object>) list1,
>> (ObservableList<Object>) list2);
>
> Although the previous code has the same problem, this is sketchy. The two
> lists can be of different types while `ListContentBinding` requires the same
> type. This is a result of the `Bindings` public API that takes two `Objects`,
> so all type information is lost. Is it worth adding a comment about this
> since suppressing the warning can be understood as "trust me, this is fine".
What would go wrong if they're not the same type? `ListContentBinding` doesn't
(and can't) enforce it and doesn't care either way. The whole function fails
silently if types don't match. Also `ListContentBinding` is a private class
and so I'd expect this code to be aware of how it works and what is/isn't safe
to do.
I personally think this entire class is unfinished. It fails miserably in edge
cases without so much as a warning to the user. Take this for example:
public static void main(String[] args) {
ObservableList<String> a = FXCollections.observableArrayList();
ObservableList<String> b = FXCollections.observableArrayList();
Bindings.bindContentBidirectional(a, b);
Bindings.bindContentBidirectional(a, b);
a.add("A");
System.out.println(a + " : " + b);
}
Prints:
[A, A, A, A] : [A, A, A]
No mention about this in the API docs at all. It breaks even worse when you
make circular bindings `[a,b], [b,c], [c,a]` (stack traces get logged to the
console complaining about `IndexOutOfBoundsException`).
I've created a solution that rejects double bindings and circular bindings, but
it's a bit out of scope for this. I think however that it is worth adding, and
considering that the current behavior is broken when doing any of such things,
not a big API break if instead we throw an exception.
-------------
PR: https://git.openjdk.org/jfx/pull/972
