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