On Tue, 9 Feb 2021 23:44:56 GMT, Kevin Rushforth <[email protected]> wrote:
>> hmm ... might appear convenient (in very controlled contexts) but looks like
>> a precondition violation: the sender of the change must not be null
>> (concededly not explicitly spec'ed but logically implied, IMO)
>>
>> so would tend to _not_ see this as blueprint for a general pattern fx code
>> base
>
> I took a quick look at your latest change, and it seems reasonable to me now
> that you are calling a helper method rather than triggering a change method
> on the listener. I'll take a closer look later in the week. In the mean time
> @kleopatra and @arapte can provide their feedback.
not going for a full review - just a comment: agree with Kevin, the delegate
methods are the way out, basically look good
`No further changes in testing required as it is all covered` - a bold
statement .. looks like there's a missed null check (which was in the listener
code but didn't make it into the delegate) at the end of sceneChanged
windowChanged(oldScene.getWindow(), newScene.getWindow());
any of old/new scene can be null (or what am I missing?) If that's not covered
by the tests .. ;)
-------------
PR: https://git.openjdk.java.net/jfx/pull/185