On Tue, 22 Oct 2024 05:54:28 GMT, John Hendrikx <jhendr...@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
