On Sat, 14 Mar 2026 02:18:24 GMT, Christopher Schnick <[email protected]>
wrote:
>> This is not an equivalent change: Before the change, a `ClassCastException`
>> could have been thrown from `observable.getValue()`, which is then swallowed
>> and logged. After the change, the exception bubbles up to the caller.
>>
>> However, the old code doesn't make any sense at all, because it doesn't do
>> what the original author intended it to do. Christopher is correct: the
>> generic type `T` doesn't have a lower bound, so it is erased to `Object` in
>> the method descriptor: `()[Ljava/lang/Object;`. The statement `return
>> (T)observable.getValue();` can therefore never throw a `ClassCastException`
>> at runtime, because any reference type is convertible to `Object`. The
>> try-catch block is dead code at best, and actually _hides_ an unrelated
>> `ClassCastException` that was thrown from `observable.getValue()`; it
>> confusingly logs that the "select-binding has wrong type", which is not what
>> has happened.
>
> Alright, so it is technically not an equivalent change, but only because the
> old implementation was wrong. However, I assume that any getValue() call
> should not throw any classcastexception if it is properly implemented. So the
> likelyhood of any consequences of this change are very minor and can only
> happen for like custom implementations that throw a classcastexception.
Thank you @crschnick and @mstr2 for clarification! I think I am ok with this
change, especially since it's effectively hidden behind `Bindings.select()`.
I was worried about a scenario like this, but it works the same in master and
this PR:
public static class A {
private String string = "string";
private int num = 55;
public String getString() {
return string;
}
public int getNum() {
return num;
}
}
@Test
public void testSelect() {
A a = new A();
ObjectBinding<String> s = Bindings.select(a, "string");
// not a string
ObjectBinding<String> i = Bindings.select(a, "num");
assertEquals("string", s.get());
assertFalse(i.get() instanceof String);
assertEquals(55, i.get());
}
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2032#discussion_r2943509627
