Closed arey closed 11 years ago
Hello Antoine.
Thanks for using and contributing to DbSetup!
I'm not really convinced by this change though. The DbSetup API is not type-safe, by design, and making Binder
generic would only give the illusion that it is, IMO. You haven't made any whange in the place where Binder
is actually used (i.e. in the Insert
class). If Binder
was generic as you suggest, the code of Insert
would have to either use a raw Binder type and ignore the raw type compiler warnings, or to make unchecked casts anyway. If I understand correctly, your goal is to avoid casting Object to (let's say) Logo
when you want to insert Logo instances using a custom Binder. Couldn't you simply use such a class?
/**
* Base class for custom binders which only accept a given type. This class is useful to avoid casts and be able to
* implement custom binders in a more natural way, but a <code>ClassCastException</code> will still be thrown at
* runtime if the actual values associated to a given column are not compatible with the generic type of this binder.
* <br/>
* Consider that this class simply allows writing
*
* <pre>
* Binder logoBinder = new GenericBinder<Logo>() {
* @Override
* protected void doBind(PreparedStatement statement, int param, Logo value) throws SQLException {
* // bind the Logo value to the statement
* }
* };
* </pre>
*
* rather than
*
* <pre>
* Binder logoBinder = new Binder() {
* @Override
* public void bind(PreparedStatement statement, int param, Object value) throws SQLException {
* Logo logo = (Logo) value;
* // bind the Logo value to the statement
* }
* };
* </pre>
*
* @param <T> the type of objects that can be bound by this Binder
*
* @author JB
*/
@SuppressWarnings("PMD.AbstractNaming") // GenericBinder is a better name than AbstractBinder
public abstract class GenericBinder<T> implements Binder {
/**
* Casts the given value to T, and delegates to {@link #doBind(java.sql.PreparedStatement, int, Object)}
* @param statement the statement to bind the parameter to
* @param param The index of the parameter to bind in the statement
* @param value The value to bind (may be <code>null</code>)
* @throws SQLException if the binding throws a {@link SQLException}
*/
@SuppressWarnings("unchecked")
@Override
public final void bind(PreparedStatement statement, int param, Object value) throws SQLException {
doBind(statement, param, (T) value);
}
/**
* Binds the given value to the given parameter in the given prepared statement.
* @param statement the statement to bind the parameter to
* @param param The index of the parameter to bind in the statement
* @param value The value to bind (may be <code>null</code>)
* @throws SQLException if the binding throws a {@link SQLException}
*/
protected abstract void doBind(PreparedStatement statement, int param, T value) throws SQLException;
}
I can add the class to DbSetup if you think this is really useful to you. But it isn't really hard to write such a class by yourself when you really want it, so I'm not sure it's a good idea to add it to DbSetup: it wouldn't be used anywhere in the DbSetup code base, except in unit tests of course. And it doesn't make the code much cleaner of safer, IMHO.
Tell me what you think. If this is the only use-case, I'd rather not make the suggested modification, and not add the above class to the codebase.
JB thanks a lot for the quickness and the quality of your answer to my poor pull request. I am using DbSetup since a few weeks.By taking a look at the unit test source code, I found the Foo cast in the FooBinder class. Thinking about typical mapper classes with from and to generics, I believe it could be enhanced. But I did not mind the reason you choose to not used generics. So you're right to not merge this change and to not add the GenericBinder you write for me. And I'm sorry for having loosing your time with my not very interesting request.
Don't worry. It's a pleasure to see that people use my little baby-project :-)
Bon week-end.
2013/8/31 Antoine Rey notifications@github.com
JB thanks a lot for the quickness and the quality of your answer to my poor pull request. I am using DbSetup since a few weeks.By taking a look at the unit test source code, I found the Foo cast in the FooBinder class. Thinking about typical mapper classes with from and to generics, I believe it could be enhanced. But I did not mind the reason you choose to not used generics. So you're right to not merge this change and to not add the GenericBinder you write for me. And I'm sorry for having loosing your time with my not very interesting request.
— Reply to this email directly or view it on GitHubhttps://github.com/Ninja-Squad/DbSetup/pull/23#issuecomment-23591710 .
Avoid cast on custom binder. Minor API changes.