RFR: 8341670: [Text, TextFlow] Public API for Text Layout Info [v18]
Michael Strauß
mstrauss at openjdk.org
Thu May 22 15:08:07 UTC 2025
On Thu, 22 May 2025 14:40:43 GMT, Andy Goryachev <angorya at 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
More information about the openjfx-dev
mailing list