RFR: 8303755: Clean up around JavacElements.getAllMembers [v2]

[Jonathan Gibbons]

On Tue, 7 Mar 2023 17:24:43 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java line 844:
>> 
>>> 842:                     return
>>> 843:                         thatRank < thisRank ||
>>> 844:                         thatRank == thisRank &&
>> 
>> (minor).  both before and after, the formulation is confusing and could benefit from parentheses or better indentation
>
> Would it be even more clear if we introduced an explicit if-else?
> 
>     int thatRank = types.rank(that.type);
>     int thisRank = types.rank(this.type);
>     if (thatRank < thisRank) {
>         return true;
>     } else if (thatRank == thisRank) {
>         return that.getQualifiedName().compareTo(this.getQualifiedName()) < 0;
>     }

IMO, that's not an improvement.

Generally, style guides suggest to break before binary operators, so I'd suggest one of

int thatRank = types.rank(that.type);
int thisRank = types.rank(this.type);
return thatRank < thisRank
    || (thatRank == thisRank
        && that.getQualifiedName().compareTo(this.getQualifiedName()) < 0);


But this is now down to a more subjective opinion.   Either way is OK.

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

PR: https://git.openjdk.org/jdk/pull/12904