On Mon, 31 Jul 2023 18:20:34 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8968:
>>
>>> 8966:
>>> 8967: for (TransitionTimer<?, ?> timer : transitionTimers) {
>>> 8968: if (timer.getProperty() == property) {
>>
>> minor: this probably works, but I'd still use `equals` here
>
> Why would this be a sensible thing to do? I'm quite explicitly comparing the
> identity of `property`, since I'm interested in finding the one property that
> I'm looking for, not potentially any property that is in some way "equal" to
> the property.
>
> For (hopefully) all property implementations, `equals` trivially works
> because it is not an overridden method and therefore falls back to an
> identity comparison. What would it even mean for a property to be equal, but
> not identical to another property?
It's up to you, but this is really standard Java practice: you use `equals`
which may defer to identity checks, unless there is a very specific reason that
you **must** have an identity check. This leaves the decision of how equality
works firmly in the hands of the class author.
For styleable properties there may not be an issue, although it is an interface
that can have 3rd party implementations. In general though, you never know when
something might be a proxy, wrapper or adapter that may not be the exact same
instance, but are considered equal for most purposes.
So if you think it must be an identity check, and you want to avoid others
thinking this is a mistake, could you add a comment with the reasoning?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1280716181
