review request: 8006505 additional updates for JSR 310 (original) (raw)

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) not implemented" There is 1 % missing to be perfect: "method T readObject(Class 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 readObject(Class 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: winxp 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 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