On Tue, 9 Feb 2021 23:44:56 GMT, Kevin Rushforth <k...@openjdk.org> 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

Reply via email to