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

Zhong Yu zhong.j.yu at gmail.com
Sun Jan 13 17:50:03 UTC 2013

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 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 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

More information about the core-libs-dev mailing list