You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@thrift.apache.org by "Bryan Duxbury (JIRA)" <ji...@apache.org> on 2010/09/08 16:56:34 UTC

[jira] Created: (THRIFT-894) Make default accessors for binary fields return byte[]; provide new accessors to get ByteBuffer version

Make default accessors for binary fields return byte[]; provide new accessors to get ByteBuffer version
-------------------------------------------------------------------------------------------------------

                 Key: THRIFT-894
                 URL: https://issues.apache.org/jira/browse/THRIFT-894
             Project: Thrift
          Issue Type: Improvement
            Reporter: Bryan Duxbury
             Fix For: 0.5


Folks have pointed out that it's not always best for users to interact with the ByteBuffers that are now backing binary fields in our structs. In truth, it seems like ByteBuffers are probably an expert means of access, with associated benefits and complexities.

I think we should generate two different sets of accessors for binary fields: a default set, named like usual, that return and take byte[], and a second set, named something like "get_buffer_for_<field>" that returns the underlying ByteBuffer. The byte[] accessors can wrap around the ByteBuffer ones as necessary.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Re: [jira] Updated: (THRIFT-894) Make default accessors for binary fields return byte[]; provide new accessors to get ByteBuffer version

Posted by Mathias Herberts <ma...@gmail.com>.

> I don't think that this test would actually fail - the byte in the first
> position of both arrays *should* be equal. However, if you were to check if
> the arrays themselves were equal, indeed, they would not be.

Sorry, got my example backward....

byte[] foo = new byte[1];
foo[0] = 'A';
struct.setXXX(foo);
byte[] bar = struct.getXXX();
foo[0] = 'B';
byte[] bar2 = struct.getXXX();
Assert.assertEquals(bar[0], bar2[0]);

this one will fail when today it succeeds.

Re: [jira] Updated: (THRIFT-894) Make default accessors for binary fields return byte[]; provide new accessors to get ByteBuffer version

Posted by Bryan Duxbury <br...@rapleaf.com>.

On Wed, Sep 8, 2010 at 12:25 PM, Mathias Herberts <
mathias.herberts@gmail.com> wrote:

> > This patch adds new getters and setters. I'd love to hear what others
> think of this change.
>
> The current (pre 0.4) semantics of the getXXX for a binary field is to
> return the content of that field, i.e. the byte[] that was set.


> ByteBuffer.capacity() returns the size of the backing array which
> might be more than the actual content of the buffer, therefore
> TBaseHelper.byteBufferToByteArray may return more data than the actual
> content of the field with no way to know what part of the returned
> byte[] contains the field's binary value.
>
> Also having setXXX(byte[]) and getXXX which returns byte[] is very
> misleading as people might think that the semantics are the same as
> before when they are not, indeed:
>
> byte[] foo = new byte[1];
> struct.setXXX(foo);
> foo[0] = 'A';
> byte[] bar = struct.getXXX();
> Assert.assertEquals(foo[0], bar[0]);
>

I don't think that this test would actually fail - the byte in the first
position of both arrays *should* be equal. However, if you were to check if
the arrays themselves were equal, indeed, they would not be.



>
> will fail when it succeeds today.
>