On Mon, 15 May 2023 09:30:04 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/provider/SHA2.java line 51:
>> 
>>> 49: 
>>> 50:     private static final int ITERATION = 64;
>>> 51:     private static final int BLOCKSIZE = 64;
>> 
>> I'm not sure if it's worth defining this. A lot of other numbers (like 56 
>> and 120) are hardcoded and it's not easy to modify this number to implement 
>> a different algorithm.
>
> This is not about modifiability but about readability. It is easier to 
> understand the code if the constants have names. Especially when there exist 
> constants with the same value that serve different purposes.

Good.

>> src/java.base/share/classes/sun/security/provider/SunEntries.java line 221:
>> 
>>> 219:         addWithAlias(p, "KeyFactory", "DSA",
>>> 220:                 "sun.security.provider.DSAKeyFactory", attrs);
>>> 221:         addWithAlias(p, "KeyFactory", "HSS/LMS", 
>>> "sun.security.provider.HSS$KeyFactoryImpl", attrs);
>> 
>> This line can be shorter. Overall, it will be nice to keep lines (including 
>> comment lines) shorter than 80 chars. If the code does look really ugly, 
>> better no more than 120 chars.
>
> I think in the 21st century (when 80 column punch cards and 80-character wide 
> screens are long gone) the 80 character line length is quite  an unreasonable 
> requirement for code (decreases readability in most cases). Still, I made the 
> lines shorter.

It's still useful when reviewing code with split panes.

We can discuss on this rule.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193972006
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193971543

Reply via email to