Updating a huge hashmap from a resultset

Question

I am using Java 7

Problem: I need to retrieve records from a database table based on a condition, orig_setting = 1. This query could return ~1000 records. Parameters from 1-7 of the Params object are set with this query.

In the next query, I need to retrieve records from the same table based on another condition, curr_setting = 1. This query could return ~900 records. Parameter 8 of Params object is set with this query.

Params object has 9 fields altogether, of which paramId maps to the the primary key of the database table.

Solution proposed by me: I have decided that when query 1 is run, all the records will be stored in a hashmap with the key being the paramId of the Params object and the value being the Params object itself

When the second query is fired, a get in performed on the hashmap based on the param id; if Param object is returned, then paramater 8 is set.

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class DisplayGrid {
public static void main(String[] args) {
 DisplayGrid displayGrid = new DisplayGrid();
 displayGrid.showGridDetails();
}
private void showGridDetails() {
 Params paramsDO = null;
 Map<Integer, Params> gridData = new HashMap<>();
// This could return ~1000 records 
// In the Params object, 8 fields out of 9 are set with one field(param_id) being the primary key
 String selectQuery = "SELECT * FROM table1 WHERE orig_setting = 1";
 try (Connection connection = DriverManager.getConnection("DB Settings");
 Statement stmt = connection.createStatement();
 ResultSet resultSet = stmt.executeQuery(selectQuery);) {
 while (resultSet.next()) {
 paramsDO = new Params();
 paramsDO.setParamId(resultSet.getInt("PARAM_ID"));
 paramsDO.setParam1(resultSet.getString("PARAM_1"));
 paramsDO.setParam2(resultSet.getString("PARAM_2"));
 paramsDO.setParam3(resultSet.getString("PARAM_3"));
 paramsDO.setParam4(resultSet.getString("PARAM_4"));
 paramsDO.setParam5(resultSet.getString("PARAM_5"));
 paramsDO.setParam6(resultSet.getString("PARAM_6"));
 paramsDO.setParam7(resultSet.getString("PARAM_7"));
// key is the primary key(param_id)
 gridData.put(new Integer(paramsDO.getParamId()), paramsDO);
 }
 } catch (Exception e) {
 throw e;
 }
// This could return ~900 records 
// from this query, the 9th field is set based on the paramId 
 selectQuery = "SELECT * FROM table1 WHERE curr_setting = 1";
 try (Connection connection = DriverManager.getConnection("DB Settings");
 Statement stmt = connection.createStatement();
 ResultSet resultSet = stmt.executeQuery(selectQuery);) {
 while (resultSet.next()) {
 paramsDO = null;
 int currParamId = resultSet.getInt("PARAM_ID");
 paramsDO = gridData.get(currParamId);
 if (paramsDO != null)
 paramsDO.setParam8(resultSet.getString("PARAM_8"));
 }
 } catch (Exception e) {
 throw e;
 }
}
private class Params {
 private int paramId; // Primary Key
 private String param1;
 private String param2;
 private String param3;
 private String param4;
 private String param5;
 private String param6;
 private String param7;
 private String param8;
 public Params() {
 super();
 }
 public Params(int paramId, String param1, String param2, String param3,
 String param4, String param5, String param6, String param7,
 String param8) {
 super();
 this.paramId = paramId;
 this.param1 = param1;
 this.param2 = param2;
 this.param3 = param3;
 this.param4 = param4;
 this.param5 = param5;
 this.param6 = param6;
 this.param7 = param7;
 this.param8 = param8;
 }
 public int getParamId() {
 return paramId;
 }
 public void setParamId(int paramId) {
 this.paramId = paramId;
 }
 public String getParam1() {
 return param1;
 }
 public void setParam1(String param1) {
 this.param1 = param1;
 }
 public String getParam2() {
 return param2;
 }
 public void setParam2(String param2) {
 this.param2 = param2;
 }
 public String getParam3() {
 return param3;
 }
 public void setParam3(String param3) {
 this.param3 = param3;
 }
 public String getParam4() {
 return param4;
 }
 public void setParam4(String param4) {
 this.param4 = param4;
 }
 public String getParam5() {
 return param5;
 }
 public void setParam5(String param5) {
 this.param5 = param5;
 }
 public String getParam6() {
 return param6;
 }
 public void setParam6(String param6) {
 this.param6 = param6;
 }
 public String getParam7() {
 return param7;
 }
 public void setParam7(String param7) {
 this.param7 = param7;
 }
 public String getParam8() {
 return param8;
 }
 public void setParam8(String param8) {
 this.param8 = param8;
 }
 @Override
 public String toString() {
 return "Params [paramId=" + paramId + ", param1=" + param1
 + ", param2=" + param2 + ", param3=" + param3 + ", param4="
 + param4 + ", param5=" + param5 + ", param6=" + param6
 + ", param7=" + param7 + ", param8=" + param8 + "]";
 }
}
}

Kindly suggest how this code can be improved to increase the performance. Also, suggest other possible solutions.

What other java data structure can I use for this purpose; could be one from other java libraries too

Posting the sample database table

+----+-----+-------+---------+---------+
| Id | Key | Value | Is_Orig | Is_Prev | 
+----+-----+-------+---------+---------+
| 1 | 10 | 1000 | 1 | 0 |
| 2 | 10 | 2000 | 0 | 0 |
| 3 | 10 | 3000 | 0 | 1 |
| 4 | 10 | 4000 | 0 | 0 |
| 5 | 20 | 100 | 1 | 0 |
| 6 | 20 | 300 | 0 | 1 |
| 7 | 20 | 400 | 0 | 0 |
| 8 | 30 | 10 | 1 | 0 |
+----+-----+-------+---------+---------+

Expected result

+-----+------------+------------+
| Key | Orig Value | Prev Value | 
+-----+------------+------------+
| 10 | 1000 | 3000 |
| 20 | 100 | 300 |
| 30 | 10 | n/a |
+-----+------------+------------+

Answer 1

there was an issue with the code, I was obligated to add the SQLException to the method, since the code was not compiling otherwise.

Before

//[...]
private void showGridDetails() {}
//[...]

After

//[...]
private void showGridDetails() throws SQLException {}
//[...]

For the review, here what I suggest:

SQL

I suggest that you put an example of the table with false data and the name / kind (MySQL, Oracle, PostgreSQL) of the database that you are using, so we can have a better understanding of it.

Code

Method showGridDetails

1) Extract the variable selectQuery into two constants, this will be better since it will make the code shorter and the string won’t be recreated each time the method is called.

public static final String FIRST_QUERY = "SELECT * FROM table1 WHERE orig_setting = 1";
public static final String SECOND_QUERY = "SELECT * FROM table1 WHERE curr_setting = 1";

2) In my opinion, when I have multiples java.lang.AutoCloseable to chain, I prefer to have multiples try.

Before

try (Connection connection = DriverManager.getConnection("DB Settings");
 Statement stmt = connection.createStatement();
 ResultSet resultSet = stmt.executeQuery(FIRST_QUERY);) {
 // [...]
} catch (Exception e) {
 throw e;
}

After

try (Connection connection = DriverManager.getConnection("DB Settings")) {
 try (Statement stmt = connection.createStatement()) {
 try (ResultSet resultSet = stmt.executeQuery(FIRST_QUERY)) {
 //[...]
 }
 }
} catch (Exception e) {
 throw e;
}

3) Instead of using the Statement and passing the parameter, I suggest that you use the java.sql.PreparedStatement. In my opinion, the java.sql.PreparedStatement is a better choice even if the parameter is hardcoded. This will offer better performances and security (SQL injection), if the parameter changes and comes from the user, external source, etc.

If you want more arguments, I suggest Difference between Statement and PreparedStatement on stackoverflow.

Examples: https://docs.oracle.com/javase/tutorial/jdbc/basics/prepared.html

try (Connection connection = DriverManager.getConnection("DB Settings")) {
 try (PreparedStatement stmt = connection.prepareStatement(FIRST_QUERY)) {
 stmt.setInt(1, 1);
 try (ResultSet resultSet = stmt.executeQuery()) {
 while (resultSet.next()) {
 paramsDO = new Params();
 paramsDO.setParamId(resultSet.getInt("PARAM_ID"));
 paramsDO.setParam1(resultSet.getString("PARAM_1"));
 paramsDO.setParam2(resultSet.getString("PARAM_2"));
 paramsDO.setParam3(resultSet.getString("PARAM_3"));
 paramsDO.setParam4(resultSet.getString("PARAM_4"));
 paramsDO.setParam5(resultSet.getString("PARAM_5"));
 paramsDO.setParam6(resultSet.getString("PARAM_6"));
 paramsDO.setParam7(resultSet.getString("PARAM_7"));
// key is the primary key(param_id)
 gridData.put(new Integer(paramsDO.getParamId()), paramsDO);
 }
 }
 }
} catch (Exception e) {
 throw e;
}

4) The creation of a new Integer is useless, since the Params#paramId is already an int.

Before

gridData.put(new Integer(paramsDO.getParamId()), paramsDO);

After

gridData.put(paramsDO.getParamId(), paramsDO);

5) Since the method is throwing the exception, both of the catch can be removed.

6) The initialisation of the variable paramsDO to null is useless, since it's updated (in the start & second query).

7) You can use the java.sql.Connection for multiples statements.

try (Connection connection = DriverManager.getConnection("DB Settings")) {
 try (PreparedStatement stmt = connection.prepareStatement(FIRST_QUERY)) {
 stmt.setInt(1, 1);
 try (ResultSet resultSet = stmt.executeQuery()) {
 while (resultSet.next()) {
 paramsDO = new Params();
 paramsDO.setParamId(resultSet.getInt("PARAM_ID"));
 paramsDO.setParam1(resultSet.getString("PARAM_1"));
 paramsDO.setParam2(resultSet.getString("PARAM_2"));
 paramsDO.setParam3(resultSet.getString("PARAM_3"));
 paramsDO.setParam4(resultSet.getString("PARAM_4"));
 paramsDO.setParam5(resultSet.getString("PARAM_5"));
 paramsDO.setParam6(resultSet.getString("PARAM_6"));
 paramsDO.setParam7(resultSet.getString("PARAM_7"));
 // key is the primary key(param_id)
 gridData.put(paramsDO.getParamId(), paramsDO);
 }
 }
 }
 // This could return ~900 records
 // from this query, the 9th field is set based on the paramId
 try (PreparedStatement stmt = connection.prepareStatement(SECOND_QUERY)) {
 stmt.setInt(1, 1);
 try (ResultSet resultSet = stmt.executeQuery()) {
 while (resultSet.next()) {
 int currParamId = resultSet.getInt("PARAM_ID");
 paramsDO = gridData.get(currParamId);
 if (paramsDO != null)
 paramsDO.setParam8(resultSet.getString("PARAM_8"));
 }
 }
 }
}

8) For the comments, I suggest that you use the block-comment (/**/) for the comments with more than one line.

Edited code

public static final String FIRST_QUERY = "SELECT * FROM table1 WHERE orig_setting = ?";
public static final String SECOND_QUERY = "SELECT * FROM table1 WHERE curr_setting = ?";
private void showGridDetails() throws SQLException {
 Params paramsDO;
 Map<Integer, Params> gridData = new HashMap<>();
 /*
 This could return ~1000 records
 In the Params object, 8 fields out of 9 are set with one field(param_id) being the primary key
 */
 try (Connection connection = DriverManager.getConnection("DB Settings")) {
 try (PreparedStatement stmt = connection.prepareStatement(FIRST_QUERY)) {
 stmt.setInt(1, 1);
 try (ResultSet resultSet = stmt.executeQuery()) {
 while (resultSet.next()) {
 paramsDO = new Params();
 paramsDO.setParamId(resultSet.getInt("PARAM_ID"));
 paramsDO.setParam1(resultSet.getString("PARAM_1"));
 paramsDO.setParam2(resultSet.getString("PARAM_2"));
 paramsDO.setParam3(resultSet.getString("PARAM_3"));
 paramsDO.setParam4(resultSet.getString("PARAM_4"));
 paramsDO.setParam5(resultSet.getString("PARAM_5"));
 paramsDO.setParam6(resultSet.getString("PARAM_6"));
 paramsDO.setParam7(resultSet.getString("PARAM_7"));
 // key is the primary key(param_id)
 gridData.put(paramsDO.getParamId(), paramsDO);
 }
 }
 }
 /*
 This could return ~900 records
 from this query, the 9th field is set based on the paramId
 */
 try (PreparedStatement stmt = connection.prepareStatement(SECOND_QUERY)) {
 stmt.setInt(1, 1);
 try (ResultSet resultSet = stmt.executeQuery()) {
 while (resultSet.next()) {
 int currParamId = resultSet.getInt("PARAM_ID");
 paramsDO = gridData.get(currParamId);
 if (paramsDO != null)
 paramsDO.setParam8(resultSet.getString("PARAM_8"));
 }
 }
 }
 }
 }
```

• Sara: Thank you for your review. Is there a better data structure apart from hashmap that can be used for this purpose?
• Doi9t: In my opinion, the best way would be to do the mapping on the database side, but if not possible, the java.util.HashMap is probably the best way to do the mapping, since the basic operations are constant time (get and put).
• Sara: I'll incorporate the suggested changes. Thank you for your inputs.

Answer 2

My first question has to be: Why Java 7? If this is production code, then your environment urgently needs to be upgraded since 7 hasn't beeen supported by Oracle for over 5 years. And if this is just practice code there is no reason not to use a more (preferably the most) current version.

---

Is this production (or at least realistic) code? It doesn't seem so, since the method doesn't return (or use) the data extracted.

---

You shouldn't be handling the database connection yourself (especially if the is supposed to be production code). At the very least you should not be opening and closing the connection for the two queries. Instead the (open) database connection (or a database connection pool) should be provided from the outside.

---

Then my next question would be: Why have two queries? It seems that both queries are supposed to return the same records (or at least the second queries returns a subset of the records from the first query). That means the first query already contains all the information needed.

So the reading of the records can be:

// Map no longer needed. Store the data in a list instead.
List<Params> gridData = new ArrayList<>();
// Get results here
while (resultSet.next()) {
 int paramId = resultSet.getInt("PARAM_ID"));
 String param1 = resultSet.getString("PARAM_1");
 // Further params omitted
 int currSetting = resultSet.getInt("curr_setting");
 String param8 = currSetting == 1 ? resultSet.getString("PARAM_8") : null;
 // Declare the paramsDO variable here, and not outsided the loop, 
 // because its the smallest needed scope.
 // Also you have the constructor so use it.
 Params paramsDO = new Params(paramId, param1, /* ..., */ param8);
 gridData.add(paramsDO);
}
// Actually do somethign with the data
return gridData;

---

Finally especially in data classes such as Params make sure you actually need setters. Even if you keep the two queries you only need setParam8 and not the others. In my example you don't need any setters at all. Immutable data objects have many advantages over mutable ones.

• Sara: This is, unfortunately, production code. I do not have any authority of choosing the version. I have edited the method for code review. Yes, there is a connection available from a utility class; I would be using that. Also, this DO object would be displayed on the UI. The method would return the map that is used in my original code.
• Sara: Both the queries return different set of records. The conditions orig_setting=1 and curr_setting=1 are mutually exclusive.
• Sara: Setter for only param8 seems sensible. Will incorporate that change.
• RoToRa: How are the result sets mutually exclusive? They query the same table, and any records that second query returns, that haven't been returned by the first query are ignored by your code. Can you post example content of the table with expected query results and expected result of your code?
• Sara: I have edit the OP to include a sample database table.

Answer 3

It is possible to join the queries into one query by having two separate queries on table1 that join on the primary key:

SELECT orig_setting_tbl.PARAM_ID, PARAM_1, PARAM_2, PARAM_3, PARAM_4, PARAM_5, PARAM_6, PARAM_7, PARAM_8
FROM
(SELECT PARAM_ID, PARAM_1, PARAM_2, PARAM_3, PARAM_4, PARAM_5, PARAM_6, PARAM_7
FROM table1 WHERE orig_setting = 1) AS orig_setting_tbl
LEFT OUTER JOIN
(SELECT PARAM_ID, PARAM_8
FROM table1 WHERE curr_setting = 1) AS curr_setting_tbl
ON orig_setting_tbl.PARAM_ID = curr_setting_tbl.PARAM_ID 

this gives you the correct complete set of params for the same PARAM_ID. all rows where orig_setting = 1 are fetched. rows that do not have matching curr_setting = 1, will have null value in PARAM_8.
I believe this will perform better than iterating over two queries.