review request: 8006505 additional updates for JSR 310
Lance Andersen - Oracle
Lance.Andersen at oracle.com
Wed Feb 6 15:20:49 UTC 2013
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
More information about the core-libs-dev
mailing list