On Mon, 24 May 2021 11:56:35 GMT, Jose Pereda <[email protected]> wrote:

> ListPropertyBase::bind, SetPropertyBase::bind, MapPropertyBase::bind have a 
> check on whether a different instance of the observable is the same, and this 
> PR changes that to check against identity.
> 
> Tests are included.

Looks good. Verified tests failing before and succeed after the fix.

One side note which might explaing, why equals was used: The super class 
`ReadOnlyList/Set/MapProperty` have hashcode/equals overridden, where equals 
accepts the corresponding collection (List/Set/Map), which is then checked 
against the collection the property has set. 

This is fine, although it looks a bit weird at first as the equals expects a 
collection, not a property. But it makes sense as all those classes implements 
the corresponding collection interface (List/Map/Set).

And a quick look in the history is showing, that at first the equals was really 
checking for a property (and in there: bean and name), which makes 
`normalList.equals(listProperty)` never work. So maybe this is leftover code 
from the old implementaation.

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

Marked as reviewed by [email protected] (no known OpenJDK username).

PR: https://git.openjdk.java.net/jfx/pull/516

Reply via email to