On Thu, 15 May 2025 20:18:04 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/common/ImageTools.java
>> line 166:
>>
>>> 164: return true;
>>> 165: }
>>> 166:
>>
>> 1. Is looking for a slash going to be compatible on all platforms? Where is
>> the path string coming from?
>> 2. Catching `NumberFormatException` to "check" if something is a number is
>> bad form
>> 3. It will allow `@0x` and `@-1x` etc...
>> 4. Consider using a regular expression, it is much more concise and intended
>> for this kind of matching
>>
>> Here's a regular expression for this:
>>
>> Pattern SCALED_PATTERN = Pattern.compile(".*@[1-9][0-9]?x(\.[^\.]+)?");
>>
>> The above will match any path that ends with `@` followed by a number from 1
>> to 99, followed by an `x`, optionally followed by an extension that does not
>> contain a dot. No need to check for slashes.
>
> If you want you can even return the scale with a slightly altered pattern:
>
> private static final Pattern SCALED_PATTERN =
> Pattern.compile(".*@([1-9][0-9]?)x(?:\.[^\.]+)?");
>
> Then do:
>
> Matcher matcher = SCALED_PATTERN.matcher(path);
>
> if (matcher.matches()) {
> return Integer.parseInt(matcher.group(1)); // can't throw
> NumberFormatException, number is validated by pattern
> }
>
> return 0; // there was no scale
>Is looking for a slash going to be compatible on all platforms? Where is the
>path string coming from?
My assumption was that other parts of ImageTools use this assumption so here it
would be okay as well. `getScaledImageName` right above this method assumes so
(although there it might not matter as much).
Regex might be a better fit for this, I agree.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1809#discussion_r2092390883
