4

I am working on an inherited large Java code base where many disparate methods make queries to the database in maddeningly different ways; as I've been debugging and standardizing everything, most of the code I write ends up looking like this:

log.info("Audit-required logging for query: "+SOME_QUERY);
log.info("Ditto for each argument: "+parameter+" "+otherParameter+ ...);
ps = conn.prepareStatement(SOME_QUERY);
ps.setString(1, aString);
ps.setString(2, anotherString);
// ... 
ps.setString(14, yetAnotherString);
rs = ps.executeQuery();
log.debug("Query executed: "+SOME_QUERY);

I hate that I have to write down the query thrice and the parameters twice (plus do a setString() for each one) — it's a recipe for future bugs when doing maintenance. I'd rather put all that inside a (static) general-purpose method that would allow me to just say everything once (plus future-proof the code base, in case some other action is needed... like, say, a different kind of logging is required by the legal department or a new kind of error handling is asked for). Something like this:

public static PreparedStatement fullyPrepare(final Connection conn, final String query, final String... arguments) { ... }

which I'd then call with a single sentence (instead of an entire code block every time):

ps = fullyPrepare(conn, CONSTANT_FOR_THIS_QUERY, parameter, otherParameter, ...);

I've, however, found resistance to this, based on the idea that "it would be bad practice". I've been trying to read up about this, but I can't find anything saying whether returning a PreparedStatement from a method that prepares it would be a good or a bad practice (as opposed to, say, the handling of ResultSet objects, like in the thread Is it Ok to Pass ResultSet?).

Why would my intended PreparedStatement preparator be a bad idea?

Community
  • 1
  • 1
Miguel Farah
  • 146
  • 1
  • 1
  • 7
  • 1
    You might want to check libraries like [DbUtils](https://commons.apache.org/proper/commons-dbutils/), [jdbc-helper](https://code.google.com/archive/p/jdbc-helper/) or [jdbi](http://jdbi.org/). They do all the work you show in the question and are nice to use when you don't need/want the whole ORM system (like Hibernate, MyBatis, jooq, etc.). (edit: fixed jdbc-helper link to the one I was thinking about, not the Jruby one) – Olivier Grégoire Feb 01 '17 at 16:37

2 Answers2

3

Doing so is a good an excellent idea.

Why is it good? Because you centralize your PrepareStatement creation in a single place. Your colleagues can read your code more cleanly. And if there's a bug, it's fixed in a single place and not everywhere.

It's so a good idea that several frameworks are even designed to do so in your place. I mentioned DbUtils, jdbc-helper and jDBI in my comment. Others are jcabi-jdbc, or the popular JOOQ. Okay, they're not all totally active (jdbc-helper may even be dead), but they're still used in production projects.

The case you mention specifically, can also be dependant on the database layer you use because some DB expect some different values for inputs like empty lists or simply null (Oracle, I'm looking at you!). So I'd strongly encourage using those libraries instead of writing my own utility methods, jDBI being my first choice as it's the one that's currently under active development.

Olivier Grégoire
  • 33,839
  • 23
  • 96
  • 137
2

Since PreparedStatement is bound to the Connection object it needs to be synchronized in order to be accessed across multiple methods which will limit your multi-user performance.

See here for more information: Reusing of a PreparedStatement between methods?

Community
  • 1
  • 1
sharath
  • 67
  • 7
  • 2
    I don't think the OP is talking about reusing the statement. – Chetan Kinger Feb 01 '17 at 16:35
  • Yes, but the question is talking about the scenario where you are no longer having the PreparedStatement local to a method. It is created at one place and passed on to another method. Which is what my concern was. – sharath Feb 03 '17 at 13:29