On Tue, 3 Jan 2023 00:24:25 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> 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.
>
> You are right, nothing would go wrong.
>
> I agree that the behavior is undesired and should be fixed in another issue.
> I was thinking of adding more specific overloads that make sense to the
> public API and deprecating the method that takes everything, making it throw.
Adding more specific unbind functions may be a good idea, but I don't think we
can change `unbind` to throw :)
I've added concept code here (https://github.com/openjdk/jfx/pull/988) that
rejects situations that would lead to lists becoming unsynced or behave badly
in other ways.
-------------
PR: https://git.openjdk.org/jfx/pull/972
