String Building

Question

Before, I had this code, sometimes it is longer:

String dbEntry = "idBldgInfo = '" + currentBldgID + "',scenario = '305',installCost='" +
installCost + "',annualSave='" + annualSave + "',simplePayback='" + simplePayback + 
"',kwhPre='" + preKWH + "',kwhPost='" + postKWH + "',co2Pre='" + preCO2e + "',co2Post='" +
postCO2e + "',mbtuPre='" + preMBtu + "',mbtuPost='" + postMBtu + "',shortDescription='" +
econ4ShortDescription + "',category='" + category + "',longDescription='" +
econ4LongDescription + "'";

I refactored it into:

String newDbEntry = 
 getDBEntryFromDBPairs(
 new DBPair("idBldgInfo", currentBldgID), new DBPair("scenario", "305"),
 new DBPair("installCost", installCost), new DBPair("annualSave", annualSave),
 new DBPair("simplePayback", simplePayback), new DBPair("kwhPre", preKWH),
 new DBPair("kwhPost", postKWH), new DBPair("co2Pre", preCO2e),
 new DBPair("co2Post", postCO2e), new DBPair("mbtuPre", preMBtu),
 new DBPair("mbtuPost", postMBtu), new DBPair("shortDescription", econ4ShortDescription),
 new DBPair("category", category), new DBPair("longDescription", econ4LongDescription));

It looks only slightly better. Is it worth changing it?

The getDBEntryFromDBPairs method is much more efficient since it uses a StringBuilder. But IDK. Any thoughts/suggestions are appreciated.

• mariosangiorgio: What are you going to do with that dbEntry? Be sure you are not going to concatenate it with other strings to build a SQL query, it is extremely risky and will lead to SQL Injections.
• jn1kk: @Mariosangiorgio All values are locally computed, the only one that is not is currentBldgID. currentBldgID is valided in the front-end.
• mariosangiorgio: What are you using to connect to the database? Usually you can create a prepared statement and then use the APIs to create the bindings between the columns in the database and the values you want to store. Moreover, why aren't you using an Object Relational Mapping framework that would take care of persistence for you?
• jn1kk: @mariosangiorgio I am trying to make the best out of the code I inherited. I have not used Hibernate before, but might be a good learning experience. Thank you.
• mariosangiorgio: Are you working on some legacy code? Be aware that introducing Hibernate in the project you will probably need to do a lot of refactoring.

Answer 1

I think your second approach is better. It is more readable and more easily maintained. I don't have much experience with databases, but lets say (sorry if its a stupid example) there is a database that causes problems for strings like "colId = colValue", and to fix it you need to remove the spaces. How many places there would be where you will go through String dbEntry = ...; statements to correct this problem? What if for a certain database you need to use : instead of =? Using data structures like this is definitely going to make your life a lot easier in such cases.

So the conclusion is, are there any such cases that you need to consider where you might need to go through your code and change the resultant string? Because changing DBPair's implementation once would be a lot easier.

Answer 2

Your first implemenattion also uses StringBuilder under the hood. See javadoc of java.lang.String:

The Java language provides special support for the string concatenation operator ( + ), and for conversion of other objects to strings. String concatenation is implemented through the StringBuilder(or StringBuffer) class and its append method

So you cannot expect an speed improvement by your second version.

• jn1kk: Well, JLS only says it may do that. So why not.
• MrSmith42: Experience shows it is done.
• jn1kk: I tried to make a test. But JIT optimizes the hell out of it, so the + concatenation turns out faster.