On Thu, 8 Aug 2024 15:45:37 GMT, Andy Goryachev <[email protected]> wrote:
>> Returning `null` seems fine from the perspective of `StyleConverter`, but it
>> makes the calling code very awkward. Remember, we ended up here because we
>> needed a way to detect whether an object would support component-wise
>> transitions. If we can't detect that without invoking `convertBack`, we
>> would need to either:
>> 1. Speculatively decompose the value without knowing whether there even are
>> any transitions defined on the node. This is bad because most of the time,
>> there will be no transitions; we will end up deconstructing many objects for
>> no reason.
>> 2. _Assume_ that an object is component-transitionable, look up all
>> potential transitions, and decompose the value; then, if we were wrong with
>> out assumption, go back to the start and try again with another code path
>> (`Interpolatable` or no transition).
>>
>> Instead, what I've implemented now is a new interface
>> `StyleConverter.WithReconstructionSupport`, which contains both methods:
>>
>> public interface WithReconstructionSupport<T> {
>> T convert(Map<CssMetaData<? extends Styleable, ?>, Object> values);
>> Map<CssMetaData<? extends Styleable, ?>, Object> convertBack(T value);
>> }
>>
>>
>> This allows us to use object deconstruction and reconstruction with a single
>> interface reference, and also allows us to detect such support very easily.
>
> +1 for `StyleConverter.WithReconstructionSupport`
>
> maybe use a shorter name, like
>
> `StyleConverter.WithReconstruction`
Clear, thanks MIchael. I would suggest `Reconstructable` as the name, which
seems more inline with interface naming (especially `Interpolatable`). Reads
nice also:
if (converter instanceof Reconstructable r) { ... }
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1711286120
