3
\$\begingroup\$

I am reading variable length messages where each message is an int + msg bytes. Messages could be any size.

Right now my strategy is to have a direct buffer that reads from the socket, which then transfers the bytes read into an array-backed nondirect buffer. However my solution has a lot of copying, some loops and just feels a bit "icky".

Is there a better solution?

Here is the read completion handler. I'm hoping it should be pretty obvious what is going on:

class DelimitedReadCompletionHandler implements CompletionHandler<Integer, Object> {
 private final AsynchronousByteChannel channel;
 private final Consumer<byte[]> onMessageCallback;
 private final ByteBuffer channelReadBuffer; // this is a direct buffer.
 private volatile ByteBuffer currentMessageBuffer;
 public DelimitedReadCompletionHandler(AsynchronousByteChannel channel,
 Consumer<byte[]> onMessageCallback,
 ByteBuffer channelReadBuffer) {
 this.channel = channel;
 this.onMessageCallback = onMessageCallback;
 this.channelReadBuffer = channelReadBuffer;
 }
 @Override
 public void completed(Integer result,
 Object attachment) {
 channelReadBuffer.flip();
 do {
 if (currentMessageBuffer == null) {
 if (channelReadBuffer.remaining() < Integer.BYTES) {
 channelReadBuffer.compact();
 break;
 }
 currentMessageBuffer = ByteBuffer.allocate(channelReadBuffer.getInt());
 }
 int copyLength = Integer.min(currentMessageBuffer.remaining(), channelReadBuffer.remaining());
 for (int i = 0; i < copyLength; i++) {
 currentMessageBuffer.put(channelReadBuffer.get());
 }
 if (!currentMessageBuffer.hasRemaining()) {
 byte[] msg = currentMessageBuffer.array();
 ForkJoinPool.commonPool().execute(() -> onMessageCallback.accept(msg));
 currentMessageBuffer = null;
 }
 if (channelReadBuffer.hasRemaining()) {
 continue;
 }
 channelReadBuffer.clear();
 break;
 } while (true);
 channel.read(channelReadBuffer, null, this);
 }
 @Override
 public void failed(Throwable exc,
 Object attachment) {
 exc.printStackTrace();
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jun 7, 2015 at 9:29
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Write some JavaDoc!

As stated above, I was very confused by your code. However, if your methods included JavaDoc, then I wouldn't be so confused by the names.

Here is an example of JavaDoc on a simple method:

/**
 Returns the sum of both of the parameters.
*/
public int add(int a, int b) {
 return a + b;
}

Note the /**? This is required for it to be valid JavaDoc.

In more complicated functions where the parameters aren't clear or the return value isn't clear, you can include something like this in your JavaDoc:

@param nameOfParameter -- description/purpose of parameter
@return -- description of return
 -- description of return
 -- description of return

You would have multiple -- description of returns if there were multiple potential return values.


There is no point in having a do/while loop where the condition being checked is true; if it were just a normal while loop, it would run atleast once anyway.

I recommend that you change your do/while to just a while as while loops are much more common.


I can not go into the way you tried to solve the problem because I am having trouble understanding your code. I believe this is just on me, but I think that having more documentation would help anyway.

answered Jul 5, 2015 at 18:37
\$\endgroup\$
2
  • \$\begingroup\$ I'm not sure whether you realise or not, but CompletionHandler is part of the JDK! I am implementing this interface so have no choice but to use those method names. Also the JavaDoc that is on the interface is sufficient I feel (not sure what other people feel). Agree about the do-while loop, i guess I was just being lazy by writing less code. \$\endgroup\$ Commented Jul 6, 2015 at 11:01
  • \$\begingroup\$ You are right; I didn't even notice CompletionHandler was part of the JDK. \$\endgroup\$ Commented Jul 6, 2015 at 17:01

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.