1
\$\begingroup\$

I have a class in which I am passing certain parameters through the constructor and then using those parameters to make one final byte array with a proper format (header + data):

public final class Frame {
  private final byte addressedCenter;
  private final byte version;
  private final Map<byte[], byte[]> keyDataHolder;
  private final long location;
  private final long locationFrom;
  private final long locationOrigin;
  private final byte partition;
  private final byte copy;

  public Frame(byte addressedCenter, byte version,
      Map<byte[], byte[]> keyDataHolder, long location, long locationFrom,
      long locationOrigin, byte partition, byte copy) {
    this.addressedCenter = addressedCenter;
    this.version = version;
    this.keyDataHolder = keyDataHolder;
    this.location = location;
    this.locationFrom = locationFrom;
    this.locationOrigin = locationOrigin;
    this.partition = partition;
    this.copy = copy;
  }

  public byte[] serialize() {
    // All of the data is embedded in a binary array with fixed maximum size 70000
    ByteBuffer byteBuffer = ByteBuffer.allocate(70000);
    byteBuffer.order(ByteOrder.BIG_ENDIAN);

    int numOfRecords = keyDataHolder.size();
    int bufferUsed = getBufferUsed(keyDataHolder); // 36 + dataSize + 1 + 1 + keyLength + 8 + 2;

    // header layout
    byteBuffer.put(addressedCenter); // byte
    byteBuffer.put(version); // byte
    byteBuffer.putInt(numOfRecords); // int
    byteBuffer.putInt(bufferUsed); // int
    byteBuffer.putLong(location); // long
    byteBuffer.putLong(locationFrom); // long
    byteBuffer.putLong(locationOrigin); // long
    byteBuffer.put(partition); // byte
    byteBuffer.put(copy); // byte

    // now the data layout
    for (Map.Entry<byte[], byte[]> entry : keyDataHolder.entrySet()) {
      byte keyType = 0;
      byte keyLength = (byte) entry.getKey().length;
      byte[] key = entry.getKey();
      byte[] data = entry.getValue();
      short dataSize = (short) data.length;

      ByteBuffer dataBuffer = ByteBuffer.wrap(data);
      long timestamp = 0;

      if (dataSize > 10) {
        timestamp = dataBuffer.getLong(2);              
      }       

      byteBuffer.put(keyType);
      byteBuffer.put(keyLength);
      byteBuffer.put(key);
      byteBuffer.putLong(timestamp);
      byteBuffer.putShort(dataSize);
      byteBuffer.put(data);
    }
    return byteBuffer.array();
  }

  private int getBufferUsed(final Map<byte[], byte[]> keyDataHolder) {
    int size = 36;
    for (Map.Entry<byte[], byte[]> entry : keyDataHolder.entrySet()) {
      size += 1 + 1 + 8 + 2;
      size += entry.getKey().length;
      size += entry.getValue().length;
    }
    return size;
  }  
}

I would like to know if it can be improved in any way. As you can see right now, I am allocating ByteBuffer with predefined size of 70000. Is there a better way by which I can allocate the size I am using while making ByteBuffer instead of using a hardcoded 70000?

\$\endgroup\$

1 Answer 1

1
+50
\$\begingroup\$

A minor aesthetics advice

You can save some typing by using the method chains over ByteBuffer:

private int getBufferCapacity(final Map<byte[], byte[]> keyDataHolder) {
    int size = 36;
    for (Map.Entry<byte[], byte[]> entry : keyDataHolder.entrySet()) {
        size += 1 + 1 + 8 + 2;
        size += entry.getKey().length;
        size += entry.getValue().length;
    }
    return size;
}

public byte[] serialize2() {
    ByteBuffer byteBuffer = ByteBuffer.allocate(getBufferCapacity(keyDataHolder))
            .order(ByteOrder.BIG_ENDIAN);
    // Use chaining:
    byteBuffer.put(addressedCenter)
            .put(version)
            .putInt(keyDataHolder.size())
            .putInt(getBufferUsed(keyDataHolder))
            .putLong(location)
            .putLong(locationFrom)
            .putLong(locationOrigin)
            .put(partition)
            .put(copy);

    for (Map.Entry<byte[], byte[]> entry : keyDataHolder.entrySet()) {
        byte keyType = 0;
        byte[] key = entry.getKey();
        byte[] value = entry.getValue(); // A map mapping is often called a
        // key/value -pair.
        byte keyLength = (byte) key.length;
        short valueLength = (short) value.length;

        ByteBuffer dataBuffer = ByteBuffer.wrap(value);
        // Short cut:
        long timestamp = valueLength > 10 ? dataBuffer.getLong(2) : 0;

        // Chaining:
        byteBuffer.put(keyType)
                .put(keyLength)
                .put(key)
                .putLong(timestamp)
                .putShort(valueLength)
                .put(value);
    }

    return byteBuffer.array();
}

Also, when you deal with maps, the conventional terminology for a mapping is a key/value-pair. For this reason, I would rewrite

byte[] key = entry.getKey();
byte[] data = entry.getValue();

to

byte[] key = entry.getKey();
byte[] value = entry.getValue();

Then, later on in the code, I would change

byte keyLength = (byte) entry.getKey().length;

with

byte keyLength = key.length;

Instead of

long timestamp = 0;

if (dataSize > 10) {
    timestamp = dataBuffer.getLong(2);              
}   

you can write an one-liner:

long timestamp = valueLength > 10 ? dataBuffer.getLong(2) : 0;

Finally, since you know your file format, you should be able to come up with a function that computes the exact size of the file for particular set of data; call it foo. Now, you would normally do:

ByteBuffer byteBuffer = ByteBuffer.allocate(foo(keyDataHolder));

Hope that helps.

\$\endgroup\$
6
  • \$\begingroup\$ thanks for your suggestion. understood most of the part. can you tell me what function you are talking about by which I can come up with the exact size that I can use to pass to allocate method of ByteBuffer? \$\endgroup\$
    – david
    Jan 24, 2017 at 17:21
  • \$\begingroup\$ @david My idea was that since you know your file format, you should be able to write a method that given the input data computes the total (and exact) number of bytes it take to serialize it. \$\endgroup\$
    – coderodde
    Jan 24, 2017 at 19:44
  • \$\begingroup\$ Yeah that is what I am confusing as I am pretty new to this ByteBuffer stuff. You mean to say I can use keyDataHolder to figure out the size I need to allocate? \$\endgroup\$
    – david
    Jan 24, 2017 at 19:54
  • \$\begingroup\$ I already have a method getBufferUsed in my question. Is this what you were talking about to get the total size estimated? \$\endgroup\$
    – david
    Jan 24, 2017 at 19:58
  • \$\begingroup\$ so it exactly same as getBufferUsed method of mine right? \$\endgroup\$
    – david
    Jan 26, 2017 at 7:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.