On Fri, 31 Dec 2021 12:53:54 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Florian Kirmaier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> JDK-8277848
>> Added missing change
>
> Why are the new listener imlementations called `BaseChangeListener` and
> `BaseInvalidationListener`, i.e. why the _Base_?
>
> Also, if you're going to the trouble of refactoring the existing listener
> implementation, have you considered merging the very similar implementations
> into a single class? You can then re-use the listener instance and save
> another object allocation in this way:
>
>
> private static class Listener<E> extends WeakReference<ListPropertyBase<E>>
> implements InvalidationListener, ListChangeListener<E>, WeakListener {
> Listener(ListPropertyBase<E> ref) {
> super(ref);
> }
>
> @Override
> public boolean wasGarbageCollected() {
> return get() == null;
> }
>
> @Override
> public void onChanged(Change<? extends E> change) {
> ListPropertyBase<E> ref = get();
> if(ref != null) {
> ref.invalidateProperties();
> ref.invalidated();
> ref.fireValueChangedEvent(change);
> }
> }
>
> @Override
> public void invalidated(Observable observable) {
> ListPropertyBase<E> ref = get();
> if (ref == null) {
> observable.removeListener(this);
> } else {
> ref.markInvalid(ref.value);
> }
> }
> }
@mstr2
Great point. The chosen name was just because I needed a name. I switched now
to the name "Listener".
I've now merged the ChangeListener with the Invalidation Listener, as you've
suggested.
The PR should now improve the performance while fixing a bug. It might even be
quite relevant because these classes are used very often.
-------------
PR: https://git.openjdk.java.net/jfx/pull/689
