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();
    }
}
\$\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.

\$\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\$
    – Cheetah
    Commented Jul 6, 2015 at 11:01
  • \$\begingroup\$ You are right; I didn't even notice CompletionHandler was part of the JDK. \$\endgroup\$
    – SirPython
    Commented Jul 6, 2015 at 17:01

Your Answer

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

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