commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1205485 - in /commons/proper/dbutils/trunk: pom.xml src/changes/changes.xml src/main/java/org/apache/commons/dbutils/handlers/BeanMapHandler.java src/test/java/org/apache/commons/dbutils/handlers/BeanMapHandlerTest.java
Date Wed, 23 Nov 2011 18:05:37 GMT
On 23 November 2011 17:45, Bill Speirs <bill.speirs@gmail.com> wrote:
> These are good questions/points...
>
> On Wed, Nov 23, 2011 at 12:35 PM, sebb <sebbaz@gmail.com> wrote:
>
>> > +
>> > +    /**
>> > +     * The Class of beans produced by this handler.
>> > +     */
>> > +    private Class<V> type;
>> > +
>> > +    /**
>> > +     * The RowProcessor implementation to use when converting rows into
>> Objects.
>> > +     */
>> > +    private RowProcessor convert;
>> > +
>> > +    /**
>> > +     * The column index to retrieve key values from. Defaults to 1.
>> > +     */
>> > +    private int columnIndex;
>> > +
>> > +    /**
>> > +     * The column name to retrieve key values from. Either columnName or
>> > +     * columnIndex will be used but never both.
>> > +     */
>> > +    private String columnName;
>> > +
>>
>> Looks like the class fields could all be final.
>>
>
> I agree... will change. Good catch!
>
>
>> > +    @Override
>> > +    protected K createKey(ResultSet rs) throws SQLException {
>> > +        return (columnName == null) ? (K) rs.getObject(columnIndex) :
>> (K) rs
>> > +                .getObject(columnName);
>>
>> This generates a warning: "Unchecked cast from Object to K".
>>
>> Is it safe to ignore this warning? If so why?
>>
>
> This same type of things pops up in a few other pieces of code where I'm
> changing things to make them generic. I'm not 100% sure what the "best
> practice" is here.
>
> I'm thinking these are safe to ignore (add @*SuppressWarnings*("*unchecked*"))
> because if the type is wrong, it will throw a cast exception and that
> should immediately alert the developer to the root cause of the issue.

In this case, yes, ignoring the warning will result in an immediate CCE.

However, there are some generics warnings that don't result in an
immediate warning.
For example, with lists, ignoring a warning may result in allowing an
incorrect entry which only appears as a CCE much later.

> Othewise, there is no way I'm aware of in java to say something like
> if(object instanceof K) where K is a generic, because it's all washed away
> with type erasure at compile time. Any thoughts here?

If the cast should work, then suppress the warning with a suitable
comment, and document that there can be a CCE.

If there is no way of knowing whether the cast should work or not,
then perhaps the addition of generics has not been done correctly or
is not appropriate.

>>
>> > +    }
>> > +
>> > +    @Override
>> > +    protected V createRow(ResultSet rs) throws SQLException {
>> > +        // TODO Auto-generated method stub
>>
>> ??? is this still TODO?
>>
>
> Nope... will remove.
>
>
>>
> Thanks for the review/feedback... always appreciated!
>
> Bill-
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message