On Mon, 30 Oct 2023 15:13:26 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review changes
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line
> 221:
>
>> 219:
>> 220: ExpectedCharacter expectedCharacter = ExpectedCharacter.DEFAULT;
>> 221: byte[] output = new byte[computePayloadSize(input)];
>
> just a thought: what if the url contains no %? in this case one can simply
> return the input string.
> I think we could return -1 from computePayloadSize() to indicate that
> condition.
The return value is not a string, it's a byte array. So we need to do the
conversion anyway, which is what this method is doing quite efficiently by now.
It's probably not useful to use another conversion method instead, at least not
in terms of performance.
> modules/javafx.graphics/src/test/java/test/com/sun/javafx/util/DataURITest.java
> line 218:
>
>> 216: assertTrue(ex.getMessage().startsWith("Incomplete"));
>> 217: }
>> 218:
>
> very minor: could we avoid this empty line please?
> (i'll re-approve if you choose to fix this)
I haven't been able to find out whether there's a recommended style in OpenJFX
with regards to empty lines at the end of a class declaration. The codebase is
very inconsistent about that...
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376670267
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376672461
