On Tue, 11 Apr 2023 07:28:36 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>>> Well, I am just following the specification here, but JavaFX itself isn't... >>> >>> We may need to make a decision here. JavaFX is not consistent itself where >>> it applies `equals` and where it uses reference equality: >> >> Alright, so this is fine for this PR. >> >> As for the equality test, we will need to revise the implementation note >> (that I wrote) to explain correctly what is used where and consider the >> changes you proposed. ReactFX uses `equals` internally, and it was very hard >> to debug why changing to its observables caused a change in the behavior, >> which is why I'm extra careful about this issue. >> My idea was to allow the user to specify their own equality checks when >> registering a listener, but that requires a lot more thought. > >> > Well, I am just following the specification here, but JavaFX itself >> > isn't... >> > We may need to make a decision here. JavaFX is not consistent itself where >> > it applies `equals` and where it uses reference equality: >> >> Alright, so this is fine for this PR. >> >> As for the equality test, we will need to revise the implementation note >> (that I wrote) to explain correctly what is used where and consider the >> changes you proposed. ReactFX uses `equals` internally, and it was very hard >> to debug why changing to its observables caused a change in the behavior, >> which is why I'm extra careful about this issue. My idea was to allow the >> user to specify their own equality checks when registering a listener, but >> that requires a lot more thought. > > Well at least you only described what JavaFX has been doing all along then. > > I am convinced that reference equality is the right way to go; ~unwanted~ > equal changes can be ignored in the change listener (or something that wraps > your change listener). An ~unwanted~ equal change would only occur when the > type involved is mutable, which you would have to ignore at your own peril -- > once you have the wrong instance, you will either see changes to the "old" > instance that you don't want, or not pick up on any direct changes to the new > instance that you decided to ignore. > > Luckily, most stuff JavaFX is doing is using immutable types. It gets in > trouble a bit with all the Set/List/Map wrappers, which is what the test > pointed out (you need to use `InvalidationListener`s there, as they still > fire on reference equality changes). @hjohn Can you merge in the latest master? Your branch is ~ 2 months out of date and that will ensure we get an updated test run. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1056#issuecomment-1512946926
