[OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers

Alexey Ivanov aivanov at openjdk.java.net
Thu Apr 1 12:08:30 UTC 2021


On Thu, 1 Apr 2021 06:04:32 GMT, Jayathirth D V <jdv at openjdk.org> wrote:

>> When `getAllPrinterNames()` returns null, the list of `printServices` is assigned a new empty array without invalidating old services which were in the array before.
>> 
>> The old print services should be invalidated.
>
> src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java line 159:
> 
>> 157:             for (int j=0; j < printServices.length; j++) {
>> 158:                 if ((printServices[j] instanceof Win32PrintService) &&
>> 159:                             (!printServices[j].equals(defaultPrintService))) {
> 
> Is this indentation right? I thought we always map newline additional conditions and not add extra indentation.

Is it not right?
I admit I don't understand what you mean by _map_ here.

When the code is written like it was:
                if ((printServices[j] instanceof Win32PrintService) &&
                    (!printServices[j].equals(defaultPrintService))) {
                    ((Win32PrintService)printServices[j]).invalidateService();
                }
it's hard to scan: it's not clear what is part of the condition and what is the statement inside the if block.

I'd prefer to write it like this:
                if ((printServices[j] instanceof Win32PrintService)
                    && (!printServices[j].equals(defaultPrintService))) {

                    ((Win32PrintService)printServices[j]).invalidateService();
                }
That is moving the operator to the continuation line which makes it obvious it is a continuation line of the condition and adding a blank line before the statement in the code. It's still not perfect, however; and it changes the author in `blame` output.

I indented the continuation line by additional 8 spaces, which is also a common practice in Java, to visually separate the condition and the statement. In fact, it's IDE that updated the formatting, I decided to keep it because it makes the code clearer.

I can revert the change to this line if you like.
Or I can just add a blank line between the condition and the statement.

What is your preference?

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

PR: https://git.openjdk.java.net/jdk/pull/3151


More information about the 2d-dev mailing list