review request: 8006505 additional updates for JSR 310

[Lance Andersen - Oracle]

On Feb 6, 2013, at 9:26 AM, Ulf Zibis wrote:

> 
> 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" ;-)

Yes I was aware of that :-)
> 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 do not have a strong preference here so to keep things simple, I will remove the message and move on.
> 
>> 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.

There is a difference between the *Impl classes which is used only by RowSet RI and these interfaces.  And in all actuality, the original RowSet authors made a mistake to not put the impl method in the com.sun.rowset namespace.


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

Thanks for including, I will revisit this later as I mentioned.

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

-------------- next part --------------

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com