Updating class javafx.beans.binding.When
nlisker at gmail.com
Fri Feb 16 14:06:58 UTC 2018
Let's start with the refactoring then. Before I submit a bug let's check
that this plan makes sense. Attached webrev for discussion.
* Main change was to the XxxCondition classes. Instead of having 4
combinations of primitives and observables I used the unification approach
that NumberConditionBuilder took with wrapping the value in a constant
observable. I also replaced these classes with a generic wrappers
XxxConditionHolders which hold the binding part of the XxxCondition class.
* I attempted to generify XxxConditionBuilders as well. The attempt is
ConditionBuilder and an example implementation was done for boolean only -
BooleanConditionBuilder2. I think that it doesn't gain much.
* Added BooleanConstant class in internal API (it was missing for some
* The handling of Numbers in the original class is a bit weird in my eyes.
The compile time return types are DoubleBinding if one or both values are
double and NumberBinding otherwise . On the other hand, the runtime
return types takes special care to return a binding class based on widening
conventions and the docs don't mention anything about that. In my change,
the runtime type is always DoubleBinding (though I kept a placeholder
if-else chain) and that saves some code. The user can always call
floatValue() etc. anyway.
Between backwards compatibility and limitations of generics this is the
best I could come up with.
* Running the tests from gradle causes some of the When tests to fail and I
don't know why, it's hard to debug. I wrote my own ad-hoc test for one of
the failed tests and it passes.
* I noticed that StringConstant extends StringExpression while all the
other classes just implement their respective ObservableXxxValue. Don't
know why, but I can align these classes to one of those choices.
On Thu, Feb 15, 2018 at 5:04 AM, Kevin Rushforth <kevin.rushforth at oracle.com
> Sorry for the delay in responding. I was traveling when this was sent and
> barely able to keep up with urgent email / tasks.
> Most of what you suggest below seems good. My only concern is whether the
> "on demand" evaluation will have any side effects. Conceptually, it seems
> like the right thing to do.
> The refactoring you propose is probably best done as a separate bug fix,
> so that we don't mix behavioral changes with refactoring, unless you think
> that the refactoring is intertwined with the fix.
> If you would like to work on a fix, that would be good. It will need to
> include new unit tests, plus ensuring that the existing unit tests pass.
> -- Kevin
> Nir Lisker wrote:
>> I was looking at https://bugs.openjdk.java.net/browse/JDK-8089579, which
>> prompted me to have a look at When. There are a few points I would like to
>> * StringConditionBuilder#otherwise(ObservableStringValue) does not check
>> for null as other condition builders do. This results in a deeper NPE
>> when StringCondition tries to register a listener to the
>> * I would like a (re)evaluation on the above bug ticket and thoughts on
>> proposal of "on demand evaluation" using a Supplier or a similar method.
>> The behavior of the intended implementation would be to evaluate 'then'
>> 'otherwise' whenever their condition is met, and only then.
>> * The class can benefit from some small refactoring, such as using
>> Objects.requireNonNull for null checks and some code reuse to reduce the
>> chance of bugs such as the missing null check of StringConditionBuilder.
>> * There are a few Javadoc corrections and some clarifications of the
>> current behavior could be beneficial as well.
>> I can work on all of the above. How to proceed?
>> - Nir
More information about the openjfx-dev