review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl

[Zhong Yu]

Why not pass the target class, like

     private <T> T getNextAttribute(Class<T> clazz) throws SQLException {
         if (++idx >= attrib.length) {
             throw new SQLException("SQLInputImpl exception: Invalid read " +
                                    "position");
         } else {
             Object attr = attrib[idx];
             if(clazz.isInstance(attr))
                 return (T)attr;
             else
                 throw new SQLException("Incorrect atribute type:
"+clazz+" vs "+attr.getClass());
         }
     }

We can generate a more appropriate error (the ClassCastException is
too general, the caller may not understand what caused it)

More importantly, the type check is done much earlier. For example, in

 xxx     private <T> T readNext() throws SQLException {
 001         T attrib = (T)getNextAttribute();
 002         lastValueWasNull = attrib == null;
 xxx         return attrib;
 xxx     }

if the wrong attribute type is requested, line 001 does not throw, so
line 002 is also executed, which is probably a bug. If line 001
throws, we won't have this bug.

Zhong Yu

On Sun, Jan 13, 2013 at 9:11 AM, Remi Forax <forax at univ-mlv.fr> wrote:
> On 01/13/2013 02:26 PM, Ulf Zibis wrote:
>>
>> Am 13.01.2013 13:33, schrieb Remi Forax:
>>>>>
>>>>> Additionally I'm wondering, whether getNextAttribute() could be
>>>>> generified.
>>>>
>>>> Yes it probably could however I do no want to do this as part of this
>>>> change
>>>
>>>
>>> I disagree, it should not be generified, getNextAttribute() should return
>>> Object and be casted at callsite,
>>> it will more clear if a ClassCastException occurs.
>>> Relying on inference to ask the compiler to insert the cast is dubious,
>>
>>
>> I must admit, I do not understand.
>>
>> What would be wrong with that? :
>>  xxx     private <T> T readNext() throws SQLException {
>>  xxx         T attrib = (T)getNextAttribute();
>>  xxx         lastValueWasNull = attrib == null;
>>  xxx         return attrib;
>>  xxx     }
>>
>>  811     public String readNString() throws SQLException {
>>  812         return readNext();
>>  813     }
>>
>> ... so why not generifying getNextAttribute() directly?
>
>
> because it's unclear in readNString() that you can have a
> ClassCastException,
> there is no cast in readNString.
>
>>
>> -Ulf
>>
>
> Rémi
>