RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

[Nir Lisker]

On Sun, 9 Apr 2023 09:04:15 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> > Also, I really wish we could make `add/removeListener` throw on `null` :)
> 
> You mean immediately? Because the `null` check is done by `ExpressionHelper` already. What I wish for is that `removeListener` would throw when removing non-existing listeners; that would expose a ton of logic errors where code is removing some listener that it thought was present, but wasn't...

I thought about throwing immediately, but you're right, it throws early enough. Not sure if the NPE should be documented on those methods or if it's obvious. And yes, throwing on non-existing listeners removal could make sense. I'm curious what breaks in JavaFX if this change is done. Could reveal some bugs maybe.

> > I still need to finish reviewing the second bug. The logic flow is a bit complicated when taking into account the many states the binding can be at (valid, observed, active...). One question I have is about the tests: why split a When test class from an already existing one in the fluent bindings test?
> 
> I mainly did that as I felt it didn't fit well in the structure of that other test, but it could be added there. I also think that big test class may benefit from splitting up; it was getting a bit unwieldy.

Yeah, `ObservableValueFluentBindingsTest` is very long.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1056#issuecomment-1501171072