On 31/08/2021 17:52, Nir Lisker wrote:
Hi, This is a continuation of the discussion that stemmed from changing bidirectional bindings to use invalidation listeners instead of change listeners [1]. The relevant issue in JBS is [2]. I have looked at removing the eager evaluation when attaching an invalidation listener. I found that tests, for example, the method BooleanPropertyBaseTest#testAddingListenerWillAlwaysReceiveInvalidationEvent (only lines 428-432 are relevant) set the following requirement: 1. Start with a property in an invalid state. 2. Attach an invalidation listener. 3. Set a value which is different than the current one. Result: an invalidation event must be sent. This means that attaching an invalidation listener must validate the property, which is (at least currently) only possible by evaluating the value. This is in contrast to the comment in the JBS that invalidation listeners should not generally force eager evaluation. It is also in contrast to this line in the class doc of ObservableValue: "Implementations in this library mark themselves as invalid when the first invalidation event occurs. They do not generate any more invalidation events until their value is recomputed and valid again." So, either the requirement set by the tests is wrong, or invalidation listeners really do need to behave according to the test and the docs missed a practical requirement, or my analysis is wrong.
What I'm thinking is that perhaps the revalidation on registering a new InvalidationListener was done intentionally as there is no mechanism to query the current valid state of an observable (#isValid). So in order to make sure that a new interested invalidation listener does not miss the fact that a property was *already* invalid, the easiest solution might have been to revalidate it upon registration of a new listener -- then again, this does not communicate this fact, it only does so when the property changes again.
An interesting solution could be to send out an Invalidation event eagerly only to a newly registered listener if said property was invalid at the time of registration; this would inform the new listener that the property was already invalid but would not notify any existing listeners (as they already know the property to be invalid).
There are more tests that fail, but the ones I looked at rely on the same concept. For example, Node_LocalToParentTransform_Test#shouldBeNotifiedWhenNodeTransforms() tests the Node#LazyTransformProperty property. Interestingly, this one is initialized with an invalid state, while Simple___Property are initialized as valid. Thoughts?
Perhaps we could consider the registration of an invalidation listener to be infrequent enough to be of little consequence, although I agree that the documentation and tests are not in agreement.
--John
