Hello,
Core libs changes (at this stage of JDK 9) only require a single reviewer.
Cheers,
-Joe
On 11/20/2014 4:45 AM, Staffan Friberg wrote:
Hi,
Anyone who can be the second Reviewer?
Thanks,
Staffan
On 11/06/2014 04:03 PM, Staffan Friberg wrote:
Hi Andrej,
Thanks for your comments. New webrev,
http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.08
Indeed more common :)
jdk/src/java.base/share$ grep -R "private final static" *|wc -l
282
jdk/src/java.base/share$ grep -R "private static final" *|wc -l
3274
//Staffan
On 11/06/2014 12:57 PM, Andrej Golovnin wrote:
Hi Staffan,
I'm not a reviewer but I have small remarks to the code style:
- the instance variable "crc" is declared bevor the class variables.
I would move it closer to the constructor.
- documentation comments should be used for the fields "crc" and
"CRC32C_POLY", e.g. instead of
// Calculated CRC-32C value
private int crc = 0xFFFFFFFF;
use this:
/**
* Calculated CRC-32C value
*/
private int crc = 0xFFFFFFFF;
- the description of the implemented algorithm should use block
comments and should be moved to the top of the class, e.g.:
public final class CRC32C implements Checksum {
/*
* This CRC-32C implementation uses the 'slicing-by-8'
algorithm described
* in the paper "A Systematic Approach to Building High
Performance
* Software-Based CRC Generators" by Michael E. Kounavis and
Frank L. Berry,
* Intel Research and Development
*/
...
In other case I get the filling that the Usafe class implements the
algorithm. :-)
- why some of the fields declared as "private transient final
static"? I would say "transient" is not needed here. Or am I missing
something? And I would use "private static final". I think this is
the preferred way in the JDK to declare a constant.
61 private transient final static Unsafe UNSAFE =
Unsafe.getUnsafe();
62
63 // Lookup tables
64 private transient final static int[][] byteTables = new
int[8][256];
65 private transient final static int[] byteTable;
66 private transient final static int[] byteTable0 =
byteTables[0];
Best regards,
Andrej Golovnin
