HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Is there a more efficient way of executing multiple SQL prepared statements at once?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
onceexecutingsqlstatementsmorewayefficientpreparedmultiplethere

Problem

I'm updating more than one table at once and, as you will see, it uses a lot of similar looking code. Is there a more efficient way of doing this, perhaps some way of combining the queries?

For context: A user submits a small form containing information about their age, gender and favorite genre of games.

String sex, age;
String[] checkboxes = request.getParameterValues("genre");

sex = request.getParameter("sex");
age = request.getParameter("age");

try(Connection con = ds.getConnection()){
    PreparedStatement updateSex = null;
    PreparedStatement updateAge = null;
    PreparedStatement updateGenre = null;

    String updateSexString = "UPDATE sex SET count=count+1 WHERE name=?";
    String updateAgeString = "UPDATE age SET count=count+1 WHERE id=?";
    String updateGenreString = "";

    for(int i = 0; i < checkboxes.length; i++){

        updateGenreString = "UPDATE genre SET count=count+1 WHERE id=?";
        updateGenre = con.prepareStatement(updateGenreString);
        updateGenre.setString(i+1, checkboxes[i].toString());
        updateGenre.executeUpdate();
        updateGenre.close();
    }

    updateSex = con.prepareStatement(updateSexString);
    updateSex.setString(1, sex);
    updateSex.executeUpdate();
    updateSex.close();

    updateAge = con.prepareStatement(updateAgeString);
    updateAge.setString(1, age);
    updateAge.executeUpdate();
    updateAge.close();

} catch(SQLException e){
    e.printStackTrace();
}

Solution

Declaring Variables and other things:

try (Connection con = ds.getConnection()) {
    PreparedStatement updateSex = null;
    PreparedStatement updateAge = null;
    PreparedStatement updateGenre = null;

    String updateSexString = "UPDATE sex SET count=count+1 WHERE name=?";
    String updateAgeString = "UPDATE age SET count=count+1 WHERE id=?";
    String updateGenreString = "";


In your current approach, multiple prepared statements are not helping anyone..
You can just put the new prepared statements you are performing into the same PreparedStatement currentStatement.

This cuts down your declarations by quite a bit. Next are the update*String variables you have here. These things are Constants. Don't declare them on method-level, they can be reasonably expected to not change. Instead these should be classwide constants:

public class ... {
    private static final String UPDATE_SEX_STRING = "UPDATE sex SET count=count+1 WHERE name=?";
    private static final String UPDATE_AGE_STRING = "UPDATE age SET count=count+1 WHERE id=?";
    // you get the gist ;)


The small and the big wtf:

String updateGenreString = "";

for(int i = 0; i < checkboxes.length; i++){

    updateGenreString = "UPDATE genre SET count=count+1 WHERE id=?";
    updateGenre = con.prepareStatement(updateGenreString);
    updateGenre.setString(i+1, checkboxes[i].toString());


Why didn't you move the assignment to the updateGenreString, that doesn't even change to the declaration?

Additionally: Why is the parameter index depending on your loop counter? That makes absolutely no sense, because in case you have more than a single checkbox, things will break with the current statement!

If there's no second parameter, what happens when you try to set it? You get a SQLException, at least per the JavaDoc.

And another one: Why is the id a String? In general using Strings as ID/UID/UUID is strongly advised against. You're losing out on fast searchability and a few other things.

The id in the age-table also seems to be a String, at least according to the method you call.

Bottom line: Your database schema seems skewed.

Code Snippets

try (Connection con = ds.getConnection()) {
    PreparedStatement updateSex = null;
    PreparedStatement updateAge = null;
    PreparedStatement updateGenre = null;

    String updateSexString = "UPDATE sex SET count=count+1 WHERE name=?";
    String updateAgeString = "UPDATE age SET count=count+1 WHERE id=?";
    String updateGenreString = "";
public class ... {
    private static final String UPDATE_SEX_STRING = "UPDATE sex SET count=count+1 WHERE name=?";
    private static final String UPDATE_AGE_STRING = "UPDATE age SET count=count+1 WHERE id=?";
    // you get the gist ;)
String updateGenreString = "";

for(int i = 0; i < checkboxes.length; i++){

    updateGenreString = "UPDATE genre SET count=count+1 WHERE id=?";
    updateGenre = con.prepareStatement(updateGenreString);
    updateGenre.setString(i+1, checkboxes[i].toString());

Context

StackExchange Code Review Q#67220, answer score: 4

Revisions (0)

No revisions yet.