Re: constructing a long from bytes

Roedy Green wrote:
 > this is an exercise for newbies.
 >
 > Here is some code that is SUPPOSED to collect 8 bytes and roll them
 > into a long. It has two serious flaws. Can you spot them?
 >
 > byte[] a = new a[8];

0. Syntax error, should be "new byte[8];"

 > // fill a with values;

1. Only works with big endian.

 >
 > long combo = 0;
 > int shiftBy = 56;

2. Should use a private final static int to initialize this.
3. Use 8*7 might be more clear than 56.

 > for ( int i = 0; i < 8; i++ )

4. Should use a.length instead of "8".

 > {
 > combo |= a[ i ] << shiftBy;

5. Byte is signed and << is a signed shift. Need to & 0xFF here somewhere.
6. << will promote a[i] to an int... and promptly wrap half the bits
into an int-size shift. Need to specify cast to long.

 > shiftBy -= 8;
 > }

7. Should consider re-using existing library code:
      ByteBuffer bb = ByteBuffer.wrap( a );
      long combo = bb.getLong();
   or
      DataInputStream dis = new DataInputStream( new
           ByteArrayInputStream( a ) );
      long combo = dis.readLong();

Of all of these, I'd say 7 is the biggest issue. Note that the two
library calls are less lines all together than your convert routine. And
ByteBuffers doesn't throw IOExceptions like DataInputStream, and will
also handle little endian formats, so it's a drop in replacement with
more flexibility.

To be fair, I missed point 0 until my IDE told me. Also, I thought <<
would shift bits into the bit bucket, not wrap. But I knew it wouldn't
be good.

class RoedysBytesToLong
{
     static void convert() throws IOException
     {
         byte[] a = {1, 2, 3, 4, 5, 6, 7, -1};

         long combo = 0;
         int shiftBy = 56;
         for( int i = 0; i < 8; i++ ) {
             combo |= (long) (0xFF & a[i]) << shiftBy;
             shiftBy -= 8;
         }
         ByteBuffer bb = ByteBuffer.wrap( a );
         long combo2 = bb.getLong();

         DataInputStream dis =
                 new DataInputStream( new ByteArrayInputStream( a ) );
         long combo3 = dis.readLong();

     }
}

Compiled and checked with debugger.