Incorrectly setting TKey causes NPE (to nobody's surprise)

[David Medinets <da...@gmail.com>]

I did this:

TKey tKey = new TKey();
tKey.setRow(row_id.getBytes());


TRange tRange = new TRange();
trange.setStart(tKey);

scan.setRange(tRange);

Iterator iterator = scan.iterator();
iterator.hasNext();

This resulted in an NPE in:

org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)

While I have no real objection to this NPE (my code is clearly
deficient), I wonder if a more cogent error message is possible.
Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

[David Medinets <da...@gmail.com>]

Adam, your answer makes sense. I replaced the TKey with Key without a problem.

On Tue, Jun 26, 2012 at 11:30 AM, Adam Fuchs <af...@apache.org> wrote:
> Mark,
>
> You're right, my answer was totally misdirected.
>
> The real reason is that TKey is not very user-friendly is that it is auto
> generated from the code in core/src/main/thrift/data.thrift. We don't intend
> for these auto-generated classes to be part of the public API. Also, we
> don't hand-code them (anymore) to make them  convenient because we are
> likely to re-generate them when we upgrade Thrift in the future. Bottom
> line: don't use classes that are in a *.thrift package in your client code.
>
> Adam
>
>
>
> On Tue, Jun 26, 2012 at 10:31 AM, Marc P. <ma...@gmail.com> wrote:
>>
>> I don't agree with that last promotion. From a usability perspective,
>> I think it would be better to either require all arguments or allow
>> them not to be set, instead of throwing an exception. This will create
>> confusion, especially with people unfamiliar with the stack, as
>> evidenced by David's question.
>>
>> On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <af...@apache.org> wrote:
>> > The tradeoff would be convenience versus complexity in the API. I would
>> > lean
>> > towards having fewer ways to create a Key.
>> >
>> > Has this debate played out
>> > before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
>> >
>> > Adam
>> >
>> >
>> >
>> > On Tue, Jun 26, 2012 at 9:17 AM, David Medinets
>> > <da...@gmail.com>
>> > wrote:
>> >>
>> >> I play 'stupid developer' fairly well. I saw something that defines a
>> >> key and started to use it. If I set row, cf, cq, and visibility then
>> >> the iterator works fine.
>> >>
>> >> Is there any reasons why default values of "" should not be provided
>> >> for cf, cq, and visibility?
>> >>
>> >> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <ma...@gmail.com> wrote:
>> >> > I realized that Mr Slacum and I addressed the concern of using
>> >> > thrift;
>> >> > however, perhaps you are doing something internally. Have you tried
>> >> > setting the stop key on the TRange just for S&Gs?
>> >> >
>> >> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com>
>> >> > wrote:
>> >> >> Why are you using that accepts the thrift key and range? They're
>> >> >> internal communication objects within accumulo. I haven't looked the
>> >> >> code directly, but they're likely contracted to be set in a
>> >> >> different
>> >> >> manner.
>> >> >>
>> >> >>
>> >> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> >> >> <da...@gmail.com> wrote:
>> >> >>> I did this:
>> >> >>>
>> >> >>> TKey tKey = new TKey();
>> >> >>> tKey.setRow(row_id.getBytes());
>> >> >>>
>> >> >>>
>> >> >>> TRange tRange = new TRange();
>> >> >>> trange.setStart(tKey);
>> >> >>>
>> >> >>> scan.setRange(tRange);
>> >> >>>
>> >> >>> Iterator iterator = scan.iterator();
>> >> >>> iterator.hasNext();
>> >> >>>
>> >> >>> This resulted in an NPE in:
>> >> >>>
>> >> >>>
>> >> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>> >> >>>
>> >> >>> While I have no real objection to this NPE (my code is clearly
>> >> >>> deficient), I wonder if a more cogent error message is possible.
>> >> >>> Should there be guard statements somewhere to ensure a valid
>> >> >>> object?
>> >
>> >
>
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

[Adam Fuchs <af...@apache.org>]

Mark,

You're right, my answer was totally misdirected.

The real reason is that TKey is not very user-friendly is that it is auto
generated from the code in core/src/main/thrift/data.thrift. We don't
intend for these auto-generated classes to be part of the public API. Also,
we don't hand-code them (anymore) to make them  convenient because we are
likely to re-generate them when we upgrade Thrift in the future. Bottom
line: don't use classes that are in a *.thrift package in your client code.

Adam



On Tue, Jun 26, 2012 at 10:31 AM, Marc P. <ma...@gmail.com> wrote:

> I don't agree with that last promotion. From a usability perspective,
> I think it would be better to either require all arguments or allow
> them not to be set, instead of throwing an exception. This will create
> confusion, especially with people unfamiliar with the stack, as
> evidenced by David's question.
>
> On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <af...@apache.org> wrote:
> > The tradeoff would be convenience versus complexity in the API. I would
> lean
> > towards having fewer ways to create a Key.
> >
> > Has this debate played out
> > before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
> >
> > Adam
> >
> >
> >
> > On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <
> david.medinets@gmail.com>
> > wrote:
> >>
> >> I play 'stupid developer' fairly well. I saw something that defines a
> >> key and started to use it. If I set row, cf, cq, and visibility then
> >> the iterator works fine.
> >>
> >> Is there any reasons why default values of "" should not be provided
> >> for cf, cq, and visibility?
> >>
> >> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <ma...@gmail.com> wrote:
> >> > I realized that Mr Slacum and I addressed the concern of using thrift;
> >> > however, perhaps you are doing something internally. Have you tried
> >> > setting the stop key on the TRange just for S&Gs?
> >> >
> >> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com>
> wrote:
> >> >> Why are you using that accepts the thrift key and range? They're
> >> >> internal communication objects within accumulo. I haven't looked the
> >> >> code directly, but they're likely contracted to be set in a different
> >> >> manner.
> >> >>
> >> >>
> >> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> >> >> <da...@gmail.com> wrote:
> >> >>> I did this:
> >> >>>
> >> >>> TKey tKey = new TKey();
> >> >>> tKey.setRow(row_id.getBytes());
> >> >>>
> >> >>>
> >> >>> TRange tRange = new TRange();
> >> >>> trange.setStart(tKey);
> >> >>>
> >> >>> scan.setRange(tRange);
> >> >>>
> >> >>> Iterator iterator = scan.iterator();
> >> >>> iterator.hasNext();
> >> >>>
> >> >>> This resulted in an NPE in:
> >> >>>
> >> >>>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
> >> >>>
> >> >>> While I have no real objection to this NPE (my code is clearly
> >> >>> deficient), I wonder if a more cogent error message is possible.
> >> >>> Should there be guard statements somewhere to ensure a valid object?
> >
> >
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

["Marc P." <ma...@gmail.com>]

I don't agree with that last promotion. From a usability perspective,
I think it would be better to either require all arguments or allow
them not to be set, instead of throwing an exception. This will create
confusion, especially with people unfamiliar with the stack, as
evidenced by David's question.

On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <af...@apache.org> wrote:
> The tradeoff would be convenience versus complexity in the API. I would lean
> towards having fewer ways to create a Key.
>
> Has this debate played out
> before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
>
> Adam
>
>
>
> On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <da...@gmail.com>
> wrote:
>>
>> I play 'stupid developer' fairly well. I saw something that defines a
>> key and started to use it. If I set row, cf, cq, and visibility then
>> the iterator works fine.
>>
>> Is there any reasons why default values of "" should not be provided
>> for cf, cq, and visibility?
>>
>> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <ma...@gmail.com> wrote:
>> > I realized that Mr Slacum and I addressed the concern of using thrift;
>> > however, perhaps you are doing something internally. Have you tried
>> > setting the stop key on the TRange just for S&Gs?
>> >
>> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com> wrote:
>> >> Why are you using that accepts the thrift key and range? They're
>> >> internal communication objects within accumulo. I haven't looked the
>> >> code directly, but they're likely contracted to be set in a different
>> >> manner.
>> >>
>> >>
>> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> >> <da...@gmail.com> wrote:
>> >>> I did this:
>> >>>
>> >>> TKey tKey = new TKey();
>> >>> tKey.setRow(row_id.getBytes());
>> >>>
>> >>>
>> >>> TRange tRange = new TRange();
>> >>> trange.setStart(tKey);
>> >>>
>> >>> scan.setRange(tRange);
>> >>>
>> >>> Iterator iterator = scan.iterator();
>> >>> iterator.hasNext();
>> >>>
>> >>> This resulted in an NPE in:
>> >>>
>> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>> >>>
>> >>> While I have no real objection to this NPE (my code is clearly
>> >>> deficient), I wonder if a more cogent error message is possible.
>> >>> Should there be guard statements somewhere to ensure a valid object?
>
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

[Adam Fuchs <af...@apache.org>]

The tradeoff would be convenience versus complexity in the API. I would
lean towards having fewer ways to create a Key.

Has this debate played out before?
http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy

Adam



On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <da...@gmail.com>wrote:

> I play 'stupid developer' fairly well. I saw something that defines a
> key and started to use it. If I set row, cf, cq, and visibility then
> the iterator works fine.
>
> Is there any reasons why default values of "" should not be provided
> for cf, cq, and visibility?
>
> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <ma...@gmail.com> wrote:
> > I realized that Mr Slacum and I addressed the concern of using thrift;
> > however, perhaps you are doing something internally. Have you tried
> > setting the stop key on the TRange just for S&Gs?
> >
> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com> wrote:
> >> Why are you using that accepts the thrift key and range? They're
> >> internal communication objects within accumulo. I haven't looked the
> >> code directly, but they're likely contracted to be set in a different
> >> manner.
> >>
> >>
> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> >> <da...@gmail.com> wrote:
> >>> I did this:
> >>>
> >>> TKey tKey = new TKey();
> >>> tKey.setRow(row_id.getBytes());
> >>>
> >>>
> >>> TRange tRange = new TRange();
> >>> trange.setStart(tKey);
> >>>
> >>> scan.setRange(tRange);
> >>>
> >>> Iterator iterator = scan.iterator();
> >>> iterator.hasNext();
> >>>
> >>> This resulted in an NPE in:
> >>>
> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
> >>>
> >>> While I have no real objection to this NPE (my code is clearly
> >>> deficient), I wonder if a more cogent error message is possible.
> >>> Should there be guard statements somewhere to ensure a valid object?
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

[David Medinets <da...@gmail.com>]

I play 'stupid developer' fairly well. I saw something that defines a
key and started to use it. If I set row, cf, cq, and visibility then
the iterator works fine.

Is there any reasons why default values of "" should not be provided
for cf, cq, and visibility?

On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <ma...@gmail.com> wrote:
> I realized that Mr Slacum and I addressed the concern of using thrift;
> however, perhaps you are doing something internally. Have you tried
> setting the stop key on the TRange just for S&Gs?
>
> On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com> wrote:
>> Why are you using that accepts the thrift key and range? They're
>> internal communication objects within accumulo. I haven't looked the
>> code directly, but they're likely contracted to be set in a different
>> manner.
>>
>>
>> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> <da...@gmail.com> wrote:
>>> I did this:
>>>
>>> TKey tKey = new TKey();
>>> tKey.setRow(row_id.getBytes());
>>>
>>>
>>> TRange tRange = new TRange();
>>> trange.setStart(tKey);
>>>
>>> scan.setRange(tRange);
>>>
>>> Iterator iterator = scan.iterator();
>>> iterator.hasNext();
>>>
>>> This resulted in an NPE in:
>>>
>>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>>>
>>> While I have no real objection to this NPE (my code is clearly
>>> deficient), I wonder if a more cogent error message is possible.
>>> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

["Marc P." <ma...@gmail.com>]

I realized that Mr Slacum and I addressed the concern of using thrift;
however, perhaps you are doing something internally. Have you tried
setting the stop key on the TRange just for S&Gs?

On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <ma...@gmail.com> wrote:
> Why are you using that accepts the thrift key and range? They're
> internal communication objects within accumulo. I haven't looked the
> code directly, but they're likely contracted to be set in a different
> manner.
>
>
> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> <da...@gmail.com> wrote:
>> I did this:
>>
>> TKey tKey = new TKey();
>> tKey.setRow(row_id.getBytes());
>>
>>
>> TRange tRange = new TRange();
>> trange.setStart(tKey);
>>
>> scan.setRange(tRange);
>>
>> Iterator iterator = scan.iterator();
>> iterator.hasNext();
>>
>> This resulted in an NPE in:
>>
>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>>
>> While I have no real objection to this NPE (my code is clearly
>> deficient), I wonder if a more cogent error message is possible.
>> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

["Marc P." <ma...@gmail.com>]

Why are you using that accepts the thrift key and range? They're
internal communication objects within accumulo. I haven't looked the
code directly, but they're likely contracted to be set in a different
manner.


On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
<da...@gmail.com> wrote:
> I did this:
>
> TKey tKey = new TKey();
> tKey.setRow(row_id.getBytes());
>
>
> TRange tRange = new TRange();
> trange.setStart(tKey);
>
> scan.setRange(tRange);
>
> Iterator iterator = scan.iterator();
> iterator.hasNext();
>
> This resulted in an NPE in:
>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>
> While I have no real objection to this NPE (my code is clearly
> deficient), I wonder if a more cogent error message is possible.
> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

[William Slacum <ws...@gmail.com>]

Is there a reason you're using the thrift classes?

On Tue, Jun 26, 2012 at 5:56 AM, David Medinets
<da...@gmail.com> wrote:
> I did this:
>
> TKey tKey = new TKey();
> tKey.setRow(row_id.getBytes());
>
>
> TRange tRange = new TRange();
> trange.setStart(tKey);
>
> scan.setRange(tRange);
>
> Iterator iterator = scan.iterator();
> iterator.hasNext();
>
> This resulted in an NPE in:
>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>
> While I have no real objection to this NPE (my code is clearly
> deficient), I wonder if a more cogent error message is possible.
> Should there be guard statements somewhere to ensure a valid object?