On Thu, 22 May 2025 14:40:43 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Instead of returning `float[]` here, this would be a good candidate to
>> return a discriminated union instead, something like:
>>
>> sealed interface CaretGeometry {
>> record Single(float x, float y, float height) implements CaretGeometry {}
>> record Split(float x1, float x2, float y, float height) implements
>> CaretGeometry {}
>> }
>>
>>
>> This has two advantages:
>> 1. It's more expressive, as you don't need to look up what the array values
>> represent.
>> 2. Instead of disciminating between the two cases by inspecting the array
>> length as you do in `PrismLayoutInfo::caretInfoAt`:
>>
>> ```java
>> Rectangle2D[] parts;
>> if (c.length == 3) {
>> ..
>> parts = ...
>> } else {
>> ...
>> parts = ...
>> }
>> ```
>> ...you can use an exhaustive switch expression instead:
>> ```
>> Rectangle2D[] parts = switch (c) {
>> case Single s -> new Rectangle2D[] {
>> new Rectangle2D(s.x() + dx, s.y() + dy, 0, s.height())
>> };
>>
>> case Split s -> new Rectangle2D[] {
>> new Rectangle2D(...),
>> new Rectangle2D(...)
>> };
>> };
>> ```
>>
>> This is easier to read, and you'll get the compiler checks for free.
>
> I would have agreed with the proposed change, if it were a public API.
>
> This is an internal method, so it was made as low level as possible. The
> return value is documented, and the consumer is another part of internal
> code. No need to make it more complicated, in my opinion.
The point is that it's less complicated to have self-describing, strongly
typed, and compiler-checked code. Your code is more complicated because it's
brittle, untyped, and less robust against refactoring.
Just to make sure that we're on the same page here: "so it was made as low
level as possible" sounds like you deliberately chose to forgo strong typing...
to achieve _what_?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2102795258
