RFR: 8332895: Support interpolation for backgrounds and borders [v18]

John Hendrikx jhendrikx at openjdk.org
Wed Aug 7 21:01:39 UTC 2024


On Wed, 7 Aug 2024 16:03:59 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Whether a style converter supports reconstruction should be a statement about the type, not a statement about the instance. For this reason, I don't prefer an instance method like `isSupportReconstruction()`. In any case, these are our options:
>> 1. Boolean parameter / Feature enum passed to constructor
>> 2. Instance method `isSupportReconstruction()`
>> 3. Annotation
>> 4. Empty marker interface
>> 5. Interface with the `convertBack` method
>> 6. Subclassing
>> 
>> I lean slightly towards the annotation, but I'm generally okay with either option.
>
> I just saw a boolean variable being instantiated from an annotation and thought "why jump through the multiple hoops?".  since there is a boolean, why not pass it directly?
> 
> it's less about memory allocation (though I would prefer to minimize that as well, but as @hjohn pointed out the difference is just a few bytes), but more about "entities must not be multiplied beyond necessity".

Although I'm not advocating for a specific change, I think looking at how `T convert(Map)` works when it was newly introduced in Java 9 should be taken into account as well.

There seem to be a couple of general approaches:

1. Have a marker that can be checked from the outside
    - Annotation (checked from the outside)
        - Unusual for this kind of check
    - Marker interface
        - Seen more often, but unusual to not put the new method there
    - Subtype 
        - Bad idea, you can only inherit once
2. Introduce a new interface with the new method
    - Defines the method and serves as the marker at the same time
3. Have a method that can be called that guards a 2nd method call
    - Doesn't matter how this is fed (constructor, annotation, override)
        -  You see this more often, but it's not a great pattern (2 method calls for the price of one...)
4. Return a special value from a method that may be unsupported
    - The most obvious (modern) candidate is to return `Optional` but is a bit unusual to indicate support/non-support
    - Throw `UnsupportedOperationException` -- although standard, I think it indicates a programmer mistake that should be avoided (in other words, encountering this exception in production code should result in a code fix)
    - Return `null`

Now the very last option (returning `null`) was the way this was handled when `T convert(Map)` was introduced in Java 9.  Even though it may not be the prettiest solution, it has two things going for it:

- It's fast (probably the fastest of the above options, although for most by just a slim margin)
- It fits in with the existing design

So, my order of preference:
- Just return `null`
- A new interface, which includes the new method
- A marker only interface
- A supportsXYZ method (regardless of how that is approached)

I don't think I could get behind any of the other options, because they either stick out like a sore thumb versus the existing design, limit future options or just perform too poorly.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1707899228


More information about the openjfx-dev mailing list