On Thu, 3 Nov 2022 18:46:36 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/PaginationSkin.java
>> line 917:
>>
>>> 915: });
>>> 916:
>>> 917:
>>> ListenerHelper.get(PaginationSkin.this).addChangeListener(getSkinnable().currentPageIndexProperty(),
>>> (src, old, cur) -> {
>>
>> so ugly...
>> @kevinrushforth
>
> Yes, I realize that by using an accessor rather than public API, getting the
> listener handler goes from:
>
>
> getListenerHelper();
>
>
> to:
>
>
> ListenerHelper.get(this);
>
>
> or, if in a nested class, to:
>
>
> ListenerHelper.get(PaginationSkin.this);
>
>
> Since you are going to use it in multiple places, it would be less "ugly" to
> cache it, either as an instance field like this:
>
>
> ListenerHelper listenerHelper = ListenerHelper.get(this);
>
>
> Or else as a local variable in the methods where you use it:
>
>
> var listenerHelper = ListenerHelper.get(PaginationSkin.this);
>
>
> Then in all places where you use it, you would just reference the
> `listenerHelper` variable.
>
> I'd probably lean towards the instance field.
I would avoid an instance field because it's already an instance field (will
waste 8 perfectly good bytes).
I suspect javac might compile these accesses away, but it needs to be verified.
Ideally, this should be a CSR and we should add the listenerHelper() method as
it's probably the best way to offer its functionality to the developer (as
opposed to having ten thousand register**() methods).
I can live with the accessor, it's ugly but bearable.
-------------
PR: https://git.openjdk.org/jfx/pull/925
