commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Speirs <bill.spe...@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 17:45:07 GMT
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.
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?

>
> > +    }
> > +
> > +    @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-

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message