On Wed, 23 Oct 2024 22:52:34 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/text/CaretInfo.java line
>> 34:
>>
>>> 32: * @since 24
>>> 33: */
>>> 34: public sealed abstract class CaretInfo permits PrismCaretInfo {
>>
>> Given the API you define, is there a reason not to make this an interface?
>
> Originally it was an interface. Then @prrace said "Why an interface ? It is
> easier to evolve a class when you want to add more info."
>
> Since this thing is sealed, it probably does not matter, and access to class
> methods is _two_ nanoseconds faster (not that's important).
>
> The only good thing about interface is the ease of mocking in testing, but we
> don't do that.
I had forgotten about that (and it doesn't matter, as you say). Carry on.
>> modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line
>> 72:
>>
>>> 70: * @return the list of {@code TextLineInfo} objects
>>> 71: */
>>> 72: public abstract List<TextLineInfo> getTextLines();
>>
>> Is the list immutable? If so (which I think it should be), can you specify
>> that?
>
> A new instance gets created for each caller, we don't care what they do to
> it. Maybe we should say that?
>
> From the API purity perspective, it could be, but an immutable list would
> incur a copy overhead, do we really need that?
>
> It also looks like there is no easy way of creating an immutable wrapper
> similar to JDK's List12, so we'd need to invent one.
>
> (Note: the original design called for an array, but List was recommended
> because of stream/etc)
You can use `Collections.unmodifiableList(List)` to wrap the list without
copying.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1816740081
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1816751055
