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?
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());
etc...
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".
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.
Gili
--
View this message in context:
http://www.nabble.com/Refactor-ByteBuffer-for-2.0--tf4042211.html#a11489137
Sent from the mina dev mailing list archive at Nabble.com.
