On 7/8/07, cowwoc <[EMAIL PROTECTED]> wrote:
Trustin Lee wrote:
>
>> 1) You should use allocateDirect() instead of allocate(int capacity,
>> boolean
>> direct) in order to stay in line with NIO's ByteBuffer
>
> I chose to use a boolean parameter because it is much easier for a
> user to choose the type of buffer in runtime (i.e. programatically).
> It is trivial to add allocateDirect of course, but I don't think it's
> really necessary.
>
I was under the impression that direct buffers are being removed in 2.0
anyway, so this method would go away right?
No. The default buffer type will be changed to non-direct. That's all.
Trustin Lee wrote:
>
> I already tried subclassing, but NIO ByteBuffer doesn't allow
> subclassing effectively. Composition is neither a good idea because
> it will make the manipulation of the buffer content inconvenient. We
> found all those new getters and putters are useful when we deal with
> various proprietary binary protocols.
I'm not sure I understand your point about composition making things
inconvenient (how?). But besides that a third option is to make the class a
helper class containing static methods that take in a ByteBuffer per method
and carry out some operation on it. Such as:
ByteBufferHelper helper = ByteBufferHelper.getInstance();
helper.fill(buffer, 0);
System.out.println(helper.getHexValue());
I think it's a matter of preference. I'd rather just call
buffer.fill() and buf.getHexDump().
You make a good point about the other menthods I mentioned. I'm sorry I
didn't notice they were already in ByteBuffer. Though I would advocate
explicitly mentioning that getHexValue() does not modify the position (the
vast majority of other methods do so it leaves one wondering).
Trustin Lee wrote:
>
>> 5) getAllocator() is confusing because it is static. Perhaps you meant to
>> call it getDefaultAllocator()?
>
> Property name 'defaultAllocator' implies that you can choose an
> alternative allocator in runtime. You can not actually and there is
> only one allocator you can use from ByteBuffer.allocate().
>
I know it is not your intention but I still find the Javadoc a bit confusing
here. The getter reads "Returns the current allocator which manages the
**allocated buffers**." The setter reads "Changes the current allocator with
the specified one to manage the **allocated buffers from now**."
The existing setter documentation "allocated buffers from now" reads a lot
like "buffers allocated from now" to me which implies that "from now on all
new buffers that are created will use this allocator" which probably isn't
what was meant.
I would recommend using the following Javadoc instead. Getter: "Returns the
allocator used by existing and new buffers", Setter: "Sets the allocator
used by existing and new buffers".
I agree with you. Let us work on fixing the documentation.
In fact if there is only one type of allocator at this time I would advocate
removing the method altogether. If the user can't do anything meaningful
with it (yet) then it should not exist and when multiple allocators are
added in the future you can bring it back.
Sounds like a good idea, but I am still not sure if we are going to
provide only one allocator when we release 2.0. ByteBuffer allocation
is known to affect the performance characteristic of a network
application, so we need to be careful.
Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
