You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2011/11/23 22:47:00 UTC
Re: svn commit: r1205609 - in /commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils:
AsyncQueryRunner.java QueryRunner.java handlers/BeanMapHandler.java
handlers/ColumnListHandler.java handlers/KeyedHandler.java handlers/ScalarHandle
On 23 November 2011 21:29, <ws...@apache.org> wrote:
> Author: wspeirs
> Date: Wed Nov 23 21:29:50 2011
> New Revision: 1205609
>
> URL: http://svn.apache.org/viewvc?rev=1205609&view=rev
> Log:
> - Removed trailing spaces
> - Added final modifiers to fields in BeanMapHandler
> - Added comments for why CCE warnings are suppressed
>
> Modified:
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/AsyncQueryRunner.java
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ColumnListHandler.java
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/KeyedHandler.java
> commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ScalarHandler.java
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/AsyncQueryRunner.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/AsyncQueryRunner.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/AsyncQueryRunner.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/AsyncQueryRunner.java Wed Nov 23 21:29:50 2011
> @@ -48,7 +48,7 @@ public class AsyncQueryRunner extends Ab
>
> /**
> * Constructor for AsyncQueryRunner that controls the use of <code>ParameterMetaData</code>.
> - *
> + *
> * @param pmdKnownBroken Some drivers don't support {@link java.sql.ParameterMetaData#getParameterType(int) };
> * if <code>pmdKnownBroken</code> is set to true, we won't even try it; if false, we'll try it,
> * and if it breaks, we'll remember not to use it again.
> @@ -60,7 +60,7 @@ public class AsyncQueryRunner extends Ab
>
> /**
> * Constructor for AsyncQueryRunner that takes a <code>DataSource</code>.
> - *
> + *
> * Methods that do not take a <code>Connection</code> parameter will retrieve connections from this
> * <code>DataSource</code>.
> *
> @@ -72,7 +72,7 @@ public class AsyncQueryRunner extends Ab
> }
>
> /**
> - * Constructor for AsyncQueryRunner that take a <code>DataSource</code> to use and controls the use of <code>ParameterMetaData</code>.
> + * Constructor for AsyncQueryRunner that take a <code>DataSource</code> and controls the use of <code>ParameterMetaData</code>.
> * Methods that do not take a <code>Connection</code> parameter will retrieve connections from this
> * <code>DataSource</code>.
> *
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/QueryRunner.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/QueryRunner.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/QueryRunner.java Wed Nov 23 21:29:50 2011
> @@ -40,7 +40,7 @@ public class QueryRunner extends Abstrac
>
> /**
> * Constructor for QueryRunner that controls the use of <code>ParameterMetaData</code>.
> - *
> + *
> * @param pmdKnownBroken Some drivers don't support {@link java.sql.ParameterMetaData#getParameterType(int) };
> * if <code>pmdKnownBroken</code> is set to true, we won't even try it; if false, we'll try it,
> * and if it breaks, we'll remember not to use it again.
> @@ -51,7 +51,7 @@ public class QueryRunner extends Abstrac
>
> /**
> * Constructor for QueryRunner that takes a <code>DataSource</code> to use.
> - *
> + *
> * Methods that do not take a <code>Connection</code> parameter will retrieve connections from this
> * <code>DataSource</code>.
> *
> @@ -62,7 +62,7 @@ public class QueryRunner extends Abstrac
> }
>
> /**
> - * Constructor for QueryRunner that takes a <code>DataSource</code> to use and controls the use of <code>ParameterMetaData</code>.
> + * Constructor for QueryRunner that takes a <code>DataSource</code> and controls the use of <code>ParameterMetaData</code>.
> * Methods that do not take a <code>Connection</code> parameter will retrieve connections from this
> * <code>DataSource</code>.
> *
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java Wed Nov 23 21:29:50 2011
> @@ -60,23 +60,23 @@ public class BeanMapHandler<K, V> extend
> /**
> * The Class of beans produced by this handler.
> */
> - private Class<V> type;
> + private final Class<V> type;
>
> /**
> * The RowProcessor implementation to use when converting rows into Objects.
> */
> - private RowProcessor convert;
> + private final RowProcessor convert;
>
> /**
> * The column index to retrieve key values from. Defaults to 1.
> */
> - private int columnIndex;
> + private final int columnIndex;
>
> /**
> * The column name to retrieve key values from. Either columnName or
> * columnIndex will be used but never both.
> */
> - private String columnName;
> + private final String columnName;
>
> /**
> * Creates a new instance of BeanMapHandler. The value of the first column
> @@ -155,7 +155,7 @@ public class BeanMapHandler<K, V> extend
> this.columnName = columnName;
> }
>
> - @SuppressWarnings("unchecked")
> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
> @Override
> protected K createKey(ResultSet rs) throws SQLException {
> return (columnName == null) ?
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ColumnListHandler.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ColumnListHandler.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ColumnListHandler.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ColumnListHandler.java Wed Nov 23 21:29:50 2011
> @@ -90,7 +90,7 @@ public class ColumnListHandler<T> extend
> *
> * @see org.apache.commons.dbutils.handlers.AbstractListHandler#handle(ResultSet)
> */
> - @SuppressWarnings("unchecked")
> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
> @Override
> protected T handleRow(ResultSet rs) throws SQLException {
> if (this.columnName == null) {
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/KeyedHandler.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/KeyedHandler.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/KeyedHandler.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/KeyedHandler.java Wed Nov 23 21:29:50 2011
> @@ -131,7 +131,7 @@ public class KeyedHandler<K> extends Abs
> * @return Object from the configured key column name/index
> * @throws SQLException if a database access error occurs
> */
> - @SuppressWarnings("unchecked")
> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
> @Override
> protected K createKey(ResultSet rs) throws SQLException {
> return (columnName == null) ?
>
> Modified: commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ScalarHandler.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ScalarHandler.java?rev=1205609&r1=1205608&r2=1205609&view=diff
> ==============================================================================
> --- commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ScalarHandler.java (original)
> +++ commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils/handlers/ScalarHandler.java Wed Nov 23 21:29:50 2011
> @@ -92,7 +92,7 @@ public class ScalarHandler<T> implements
> *
> * @see org.apache.commons.dbutils.ResultSetHandler#handle(java.sql.ResultSet)
> */
> - @SuppressWarnings("unchecked")
> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
Sorry to keep going on about this, but that does not fully explain why
it is safe to ignore the unchecked cast.
And it does nothing to tell the user of the API that they may get a
CCE or what they need to do to avoid it.
For example:
Properties prop = new Properties();
prop.load(...);
@SuppressWarnings("unchecked") // names are Strings
Enumeration<String> propertyNames = (Enumeration<String>) prop.propertyNames();
Properties is a Hashtable<Object,Object> (for historical reasons) even
though the keys will always be strings (the load() method ensures
this).
In this case, what ensures that the cast will succeed?
Does that condition always hold?
If not, the Javadoc needs to say under what conditions the CCE will occur.
> public T handle(ResultSet rs) throws SQLException {
>
> if (rs.next()) {
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1205609 - in /commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils:
AsyncQueryRunner.java QueryRunner.java handlers/BeanMapHandler.java
handlers/ColumnListHandler.java handlers/KeyedHandler.java handlers/ScalarHandle
Posted by sebb <se...@gmail.com>.
On 25 November 2011 17:59, Bill Speirs <bi...@gmail.com> wrote:
> On Fri, Nov 25, 2011 at 12:28 PM, sebb <se...@gmail.com> wrote:
>>> As we discussed before, this exception will be thrown if the column
>>> type doesn't match the generic type. Because of type erasure I don't
>>> know of any way to validate the types.
>>
>> What determines the column type?
>
> As far as I know, the column type is determined by the table schema
> for the datbase, and the JDBC driver.
>
>> If the user codes correctly to the API, is it possible for the CCE to be thrown?
>
> If the user properly matches the generic with what is in the table
> schema, a CCE should not be thrown.
>
>> Part of the idea of generics is to do away with unnecessary casts and
>> instanceof checks; adding inappropriate @SuppressWarnings tags negates
>> the benefit (and can even make things worse).
>
> Because we only have the getObject method (there are other JDBC
> methods; however, we cannot pick the correct method because of type
> erasure), there will need to be a cast. However, this cast is done by
> the library, so the user is not forced to cast.
OK. So we can document as follows (or similar)
* @throws CCE if the class datatype does not match the column type
// We assume that the user has picked the correct type to match the column
// so getObject will return the appropriate type and the cast will succeed.
@SuppressWarnings...
> Bill-
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1205609 - in /commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils:
AsyncQueryRunner.java QueryRunner.java handlers/BeanMapHandler.java
handlers/ColumnListHandler.java handlers/KeyedHandler.java handlers/ScalarHandle
Posted by Bill Speirs <bi...@gmail.com>.
On Fri, Nov 25, 2011 at 12:28 PM, sebb <se...@gmail.com> wrote:
>> As we discussed before, this exception will be thrown if the column
>> type doesn't match the generic type. Because of type erasure I don't
>> know of any way to validate the types.
>
> What determines the column type?
As far as I know, the column type is determined by the table schema
for the datbase, and the JDBC driver.
> If the user codes correctly to the API, is it possible for the CCE to be thrown?
If the user properly matches the generic with what is in the table
schema, a CCE should not be thrown.
> Part of the idea of generics is to do away with unnecessary casts and
> instanceof checks; adding inappropriate @SuppressWarnings tags negates
> the benefit (and can even make things worse).
Because we only have the getObject method (there are other JDBC
methods; however, we cannot pick the correct method because of type
erasure), there will need to be a cast. However, this cast is done by
the library, so the user is not forced to cast.
Bill-
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1205609 - in /commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils:
AsyncQueryRunner.java QueryRunner.java handlers/BeanMapHandler.java
handlers/ColumnListHandler.java handlers/KeyedHandler.java handlers/ScalarHandle
Posted by sebb <se...@gmail.com>.
On 25 November 2011 16:55, Bill Speirs <bi...@gmail.com> wrote:
> On Wed, Nov 23, 2011 at 4:47 PM, sebb <se...@gmail.com> wrote:
>>> - @SuppressWarnings("unchecked")
>>> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
>>
>> Sorry to keep going on about this, but that does not fully explain why
>> it is safe to ignore the unchecked cast.
>> And it does nothing to tell the user of the API that they may get a
>> CCE or what they need to do to avoid it.
>>
>> In this case, what ensures that the cast will succeed?
>> Does that condition always hold?
>> If not, the Javadoc needs to say under what conditions the CCE will occur.
>>
>
> As we discussed before, this exception will be thrown if the column
> type doesn't match the generic type. Because of type erasure I don't
> know of any way to validate the types.
What determines the column type?
If the user codes correctly to the API, is it possible for the CCE to be thrown?
> Sebb, are you advocating putting an @throws ClassCastException in the
> Javadocs stating that if the column type doesn't match the generic
> type, then this exception will be thrown?
Yes.
The idea is that there should not be any surprise exceptions for
conditions that can occur in normal use.
That's where the Javadoc comes in.
Also for the maintainer of the code to understand why the warning was
suppressed, which is where the local comment is used.
Part of the idea of generics is to do away with unnecessary casts and
instanceof checks; adding inappropriate @SuppressWarnings tags negates
the benefit (and can even make things worse).
So each warning needs to be considered carefully, and any decision to
add suppression needs to be documented with the reasoning.
> What would others like to see for documentation here?
>
> Thanks!
>
> Bill-
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1205609 - in /commons/proper/dbutils/trunk/src/main/java/org/apache/commons/dbutils:
AsyncQueryRunner.java QueryRunner.java handlers/BeanMapHandler.java
handlers/ColumnListHandler.java handlers/KeyedHandler.java handlers/ScalarHandle
Posted by Bill Speirs <bi...@gmail.com>.
On Wed, Nov 23, 2011 at 4:47 PM, sebb <se...@gmail.com> wrote:
>> - @SuppressWarnings("unchecked")
>> + @SuppressWarnings("unchecked") // cast exception will immediately be thrown and warn the developer
>
> Sorry to keep going on about this, but that does not fully explain why
> it is safe to ignore the unchecked cast.
> And it does nothing to tell the user of the API that they may get a
> CCE or what they need to do to avoid it.
>
> In this case, what ensures that the cast will succeed?
> Does that condition always hold?
> If not, the Javadoc needs to say under what conditions the CCE will occur.
>
As we discussed before, this exception will be thrown if the column
type doesn't match the generic type. Because of type erasure I don't
know of any way to validate the types.
Sebb, are you advocating putting an @throws ClassCastException in the
Javadocs stating that if the column type doesn't match the generic
type, then this exception will be thrown? What would others like to
see for documentation here?
Thanks!
Bill-
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org