On Wed, 29 Nov 2023 02:44:31 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
> What each class brings with itself is a list of its own metadata properties.
> That's all it should declare. This list should automatically be added to the
> one of its parent, recursively, forming the final list. This part shouldn't
> be done by users as it's just another place to make a mistake in.
I definitely always found the implementation for gathering CSS properties
rather ugly. The problem however is in part that you can't define a method that
only provides the CSS properties of the current class and have it be part of
`Node` or some interface for the CSS subsystem to call. Such a method
implementation would have to call `super` itself, as an external caller can't
call super methods to gather the full list.
Lacking that it would have to be a static method with a specific signature that
needs to be discovered via reflection, and then go up the hierarchy to find all
properties. A static method however won't work for Skin based CSS properties.
Still, there are IMHO better ways to do this; IMHO `Node` should just have a
dedicated list of all CSS properties (exposed via `getCssMetaData`) stored in a
`CopyOnWriteArrayList` to which the subclasses contribute additional properties
at construction time. The only writes to this list occur at construction time
and when Skin changes happen, so this would be an almost ideal usecase for
`CopyOnWriteArrayList`. The `getCssMetaData` method could then have been
`final`:
Node:
private final List<CssMetaData> cssProps = new
CopyOnWriteArrayList<>();
private final List<CssMetaData> cssPropsUnmodifiable =
Collections.unmodifiableList(cssProps);
protected final int contributeProperties(List<CssMetaData> props) {
cssProps.addAll(props);
return cssProps.size();
}
protected final void replaceProperties(int fromIndex,
List<CssMetaData> props) {
cssProps.sublist(fromIndex, cssProps.size()).clear();
cssProps.addAll(props);
}
public final List<CssMetaData> getCssMetaData() { return
cssPropsUnmodifiable; }
Subclasses simply call `contributeProperties` and they're part of the
"immutable" list automatically. `Control` subclasses are a bit more advanced
as they first call `contributeProperties` with their fixed properties, store
the returned index, and then call `replaceProperties` each time their `Skin`
changes. The `Skin` properties are always at the end, so clearing and
re-adding them is easy.
No messing around with concatenations, static methods, overrides that are not
calling `super`, etc, and just as performant as there are no object allocations
involved when calling `getCssMetaData`.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1296#issuecomment-1831367054
