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

[Jayathirth D V]

On Thu, 1 Apr 2021 12:05:30 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> 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?

By mapping i mean same indentation for all conditions in if statement without adding additional indentation for each continuation line like(Basically line without your change of indentation)
if ((condition1) &&
     (condition2)) {
}

or 

if ((condition1)
     &&(condition2)) {
}

I have not come across code in java.desktop where we add indentation at each continuation line of 'if' condition.
I understand difficulty to scan without indentation but then in cases where we have multiple lines on continuation line in if statement we will easily hit 80 characters limit.

If we want to differentiate between if conditions and actual statement execution to improve readability, we can move the statement block to new line like 
if ((condition1) &&
     (condition2))
{
}

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

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