RFR JDK-6321472: Add CRC-32C API
Staffan Friberg
staffan.friberg at oracle.com
Thu Nov 6 15:03:50 UTC 2014
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
More information about the core-libs-dev
mailing list