OpenGamma / ElSql

Manage external SQL files in Java with a little DSL goodness
Other
101 stars 26 forks source link

Prefer ElSql#getSql(String, Map<String, ?>) over ElSql#getSql(String, Map<String, Object>) #13

Closed zimmi closed 9 years ago

zimmi commented 9 years ago

Essentially, this is my code:

ElSql elSql = ElSql.parse(ElSqlConfig.DEFAULT, Main.class.getResource("/sql/queries.elsql"));
Map<String, String> params = Collections.singletonMap("key", "value");
String sql = elSql.getSql("SomeQuery", params);

That won't compile, since ElSql#getSql explicitly takes a Map<String, Object>. In this case making it

Map<String, Object> params = Collections.singletonMap("key", "value");

would be an obvious fix, but that Map might come from somewhere else and casting is somewhat ugly.

Changing the signature of ElSql#getSql to take a Map<String, ?> instead would fix this minor issue in a backward compatible way, or am I missing something?

Oh, and many thanks for this awesome library, it really fills a niche. I'll try to use it together with fluent-jdbc, that seems to use the same syntax for named parameters as the Spring JdbcTemplate.

jodastephen commented 9 years ago

Changing the type parameters of a method may be source compatible, but is not binary compatible.

In Java 8, the following compiles:

Map<String, Object> params = Collections.singletonMap("key", "value");

In Java 7 I suspect you have to do this

Map<String, Object> params = Collections.<String, Object>singletonMap("key", "value");

Given all this, I'm not sure its sensible to make a change to ElSql.

BTW we use JDBI now for the SQL side, but fluent-jdbc looks interesting too.

zimmi commented 9 years ago

I learned something new, thanks a lot for the information and the link!

In that case I agree that it's silly to break binary compatibility over this. Most of the time the parameters will be of mixed types, so we're back to Object anyway.

I'll close this issue then.

Have a nice weekend!