On 05/02/2016 05:07 PM, Claes Redestad wrote:
> I'd like to sponsor this patch proposed by Jaroslav Kameník here:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040660.html
> 
> Bug https://bugs.openjdk.java.net/browse/JDK-8155795
> Webrev: http://cr.openjdk.java.net/~redestad/8155795/webrev.01/

*) I wonder if Integer.reverseBytes is better spelled as:

 return (i >>> 24) |
        ((i & 0xFF0000) >> 8) |
        ((i & 0xFF00) << 8)   |
        (i << 24);

*) The test should probably follow the same randomized testing pattern
as other tests:

 for (int i = 0; i < N; i++) {
   int x = rnd.nextInt();

   String expected = new StringBuilder().
       .append(leftpad(Integer.toBinaryString(x), 32))
       .reverse().toString();

   String actual   =
       leftpad(Integer.toBinaryString(Integer.reverse(x)), 32);

   if (!expected.equals(actual)) {
       throw new RuntimeException("reverse: \n" +
                          expected + " \n" + actual);
   }

 // That's how development looks like in 2016.
 private static String leftpad(String s, int width) {
   String r = s;
   for (int c = 0; c < width - s.length(); c++) {
     r = "0" + r;
   }
   return r;
 }

...and should probably precede any other test that uses reverse().

Thanks,
-Aleksey

Reply via email to