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

Ulf Zibis Ulf.Zibis at CoSoCo.de
Wed Feb 6 14:26:02 UTC 2013

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" ;-) 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 readObject(Class 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 :

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

More information about the core-libs-dev mailing list