On Thu, 12 Mar 2026 18:05:56 GMT, Andy Goryachev <[email protected]> wrote:
>> `CssParser.colorValueOfString()` parses a string into a `Color`. If this
>> fails by throwing an exception, `null` is returned from
>> `colorValueOfString()`, signalling to the caller that the color string might
>> be a lookup instead.
>>
>> Since color lookups can appear in many places in a CSS file, it is
>> preferable to use return values for flow control instead of exceptions. For
>> this purpose, we need non-throwing versions of methods that parse numbers
>> and colors.
>>
>> These are the changes in this PR:
>> 1. Move color parsing from the `Color` class to `CssColorParser`. This is
>> done so that we can access the new `tryParseColor(String)` method from
>> `CssParser`, and also to separate concerns between color representation and
>> parsing.
>> 2. Add a non-throwing `CssNumberParser.tryParseDouble()`, which returns
>> `NaN` to indicate a parsing failure. We can use `NaN` because it is not a
>> valid return value of the parser.
>> 3. Since color parsing also uses `Integer.parseInt()`, we need a
>> non-throwing version of this method: `CssNumberParser.tryParseInt()`. Since
>> we can't use any particular `int` as a return value that indicates failure,
>> I've decided to return a `long` value instead, where a value less than
>> `Integer.MIN_VALUE` indicates a parsing failure.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java
> line 40:
>
>> 38: /**
>> 39: * Parses a number as an integer. If successful, returns the integer
>> as a long value; otherwise
>> 40: * returns a long value less than {@link Integer#MIN_VALUE} to
>> indicate failure.
>
> minor:
> can it return anything other than `Long.MIN_VALUE` on failure?
> should we explicitly mention `Long.MIN_VALUE` then?
It doesn't return anything other than `MIN_VALUE` on failure, but keep in mind
that this really is only a simple, internal method that's only used in a single
place (it even says so in the name of the class). So for this reason, I'm going
to take the win (thanks for the review!) and defer further polish to the next
time we'll talk about this method, which is probably never.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2093#discussion_r2926703725
