review request: 8006505 additional updates for JSR 310

[Ulf Zibis]

Am 06.02.2013 13:15, schrieb Lance Andersen - Oracle:
> I am going to change the message from
>
> "readObject not implemented"
>
> to "method readObject(Class<T>) not implemented"

There is 1 % missing to be perfect: "method <T> T readObject(Class<T> type) not implemented" ;-)
I was also thinking about this 100 % message, but I thought, exception messages should not be so 
talkative/verbose. Additionally reduces class footprint and CPU cycles.

> As it was suggested to make it stand out from the other readObject method from another reviewer.

And thinking again, I think, that:
     "SQLFeatureNotSupportedException at package.MyClass.callsite(MyClass.java:123)
would be clear enough, no big difference between "...NotSupported..." and "... not implemented".

In any case, the developer has to look at the call site at MyClass.java:123, where he clearly will 
see, which method is not supported/implemented, ... so empty exception message should suffice here.

> I am just going to make the change then push later today
>>>> Have you observed internal review ID of 2426775?
>>> I have not seen this come through as of yet but will let you know when I do.
>> It was from 16.01.2013 15:19 +0100
> That was for your comment with the SQLInput/OuputImpl classes:
>
>
> ... and starting idx by 0 instead -1 would not be so exotic

This was only an aside remark, the main justification for my bug report is, that I still think, 
method <T> T readObject(Class<T> type) is completely superfluous, if we generify the existing method.

> I have not seen the issue in our internal tracking system (but have not gone looking for it either).  And as I mentioned in my response to your suggestion, I will look at this after I wrap up other work for JDBC 4.2 and RowSet 1.2

OK, I attach the report here:

Date Created: Wed Jan 16 07:19:27 MST 2013
Type:        rfe
Customer Name:   Ulf Zibis
Customer Email:Ulf.Zibis at CoSoCo.de
SDN ID:
status:      Waiting
Category:    java
Subcategory: jdbc
Company:     CoSoCo
release:     7
hardware:    x86
OSversion:   win_xp
priority:    4
Synopsis:    Generic accessors to SQLInput, SQLOutput
Description:
  A DESCRIPTION OF THE REQUEST :
Currently, interfaces SQLInput, SQLOutput provide plenty accessors for distinct type, and one general read/writeObject().
The latter could be generified to cover all other non-primitive types, with the effect pushing the root of a possible ClassCastException to the call site layer.
The non-primitive distinct ones could be deprecated.

JUSTIFICATION :
- general convenience
- save the additional external cast for readObject()
- in the javax.sql.rowset.serial implementation, the dictinct accessors could internally cause a ClassCastException, which would be unfathomable for users
- less code to maintain, especially for future extensions


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
E.G.:
boolean bb = sqlInput.readBoolean();
Boolean b = sqlInput.readObject();
String  s = sqlInput.readObject();
Blob    bl = sqlInput.readObject();
...

ACTUAL -
E.G.:
boolean bb = sqlInput.readBoolean();
Boolean b = (Boolean)sqlInput.readObject();
String  s = sqlInput.readString();
Blob    bl = sqlInput.readBlob();
...


---------- BEGIN SOURCE ----------
proposed source changes for javax.sql.rowset.serial.SQLInputImpl

     private int idx = 0; // init with 0 instead -1

     private Object attribs[]; // rename attrib to attribs

     public boolean readBoolean() throws SQLException {
         Boolean attrib = readObject(); // replace old (Boolean)getNextAttribute()
         return  (attrib == null) ? false : attrib.booleanValue();
     }

     public <T> T readObject() throws SQLException {
         if (idx >= attribs.length) {
             throw new SQLException("SQLInputImpl exception: Invalid read " +
                                    "position");
         }
         T attrib = (T)attribs[idx++];
         lastValueWasNull = attrib == null;
         if (attrib instanceof Struct) {
             ... // maybe move to extra method to enhance possibility
             ... // for JIT to compile + inline the main path
             ...
         }
         return attrib;
     }


---------- END SOURCE ----------
workaround:
comments:    (company - CoSoCo , email -Ulf.Zibis at CoSoCo.de)



-Ulf