RFR: 8306707: Support pluggable image loading via javax.imageio [v6]

[Andy Goryachev]

On Tue, 22 Oct 2024 05:54:28 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/javax/XImageLoader.java line 209:
>> 
>>> 207:                     : ImageStorage.ImageType.PALETTE;
>>> 208: 
>>> 209:                 var scanlineStride = switch(image.getSampleModel()) {
>> 
>> Just a general comment here, on the use of `var`; in my opinion, `var` helps the writer of the code, but almost never helps the reader of the code who now must read the RHS and manually determine what the type is.  As we should be aiming for maintainable code and code that's as easy to read as possible without having to guess (there's usually enough guessing involved already), I always find it hard how the use of `var` can be justified anywhere. 
>> 
>> Also in my experience, `var` complicates refactors as it will morph with the target type, making all refactor errors occur at the use location instead of the declaration site when `var` isn't used (ie. a refactor could indicate dozens of errors, while fixing 2 declarations could solve all of them).
>> 
>> So why make us guess that `scanlineStride` is an `int` here?  And why force us to read the right hand sides of `colorModel`, `palette` and `imageType`...
>
> `scanlineStride` is useful for all the `ImageFrame` constructors, or do you think recomputing it (with the risk of it being different) is still the best course of action given that AWT supplies it and might be using some other alignment/padding?

> `var` helps the writer of the code, but almost never helps the reader of the code

I fully agree with John on this.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1810924383