There are three methods that differ literally in a couple of lines. Can I somehow avoid duplicate code?

@Override public List<Employee> findAll() { List<Employee> list = new ArrayList<>(); Connection connection = null; try { connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(SQL_FIND_ALL); ResultSet rs = statement.executeQuery(); while (rs.next()) { Employee tempEmployee = convertRowToEmployee(rs); list.add(tempEmployee); } } catch (SQLException e) { e.printStackTrace(); } finally { dataSource.closeConnection(connection); } return list; } @Override public List<Employee> findByName(String name) { List<Employee> list = new ArrayList<>(); Connection connection = null; try { connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(SQL_FIND_BY_NAME); statement.setString(1, name); ResultSet rs = statement.executeQuery(); while (rs.next()) { Employee tempEmployee = convertRowToEmployee(rs); list.add(tempEmployee); } } catch (SQLException e) { e.printStackTrace(); } finally { dataSource.closeConnection(connection); } return list; } @Override public List<Employee> findAllAbsenceEmployeeByDate(Date startDate, Date endDate) { List<Employee> list = new ArrayList<>(); Connection connection = null; try { connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(SQL_FIND_ALL_ABSENCE_BY_DATE); statement.setDate(1, startDate); statement.setDate(2, endDate); ResultSet rs = statement.executeQuery(); while (rs.next()) { Employee tempEmployee = convertRowToEmployee(rs); list.add(tempEmployee); } } catch (SQLException e) { e.printStackTrace(); } finally { dataSource.closeConnection(connection); } return list; }

Converting rows into an object rendered into a separate method convertRowToEmployee (rs), merging and closing rendered into a separate class DataSource

    2 answers 2

    It is not necessary to optimize better, since then in the case of expansion (adding new features) you will have to re-test everything according to the full program, plus you will lose in the code readability. Also you have an error in the code. It is necessary to close PreparedStatement and ResultSet. I recommend the method of closing these objects also implemented in the DataSource something like this:

     public void close(){ if (conn != null){ try { conn.close(); } catch (SQLException e) { // TODO Auto-generated catch block e.printStackTrace(); } } } public void close(PreparedStatement ps){ if (ps != null){ try { ps.close(); } catch (SQLException e) { // TODO Auto-generated catch block e.printStackTrace(); } } } public void close(CallableStatement cs){ if (cs != null){ try { cs.close(); } catch (SQLException e) { // TODO Auto-generated catch block e.printStackTrace(); } } } public void close(PreparedStatement ps, ResultSet rs){ close(ps); if (rs != null){ try { rs.close(); } catch (SQLException e) { // TODO Auto-generated catch block e.printStackTrace(); } } } public void close(CallableStatement cs, ResultSet rs){ close(cs); if (rs != null){ try { rs.close(); } catch (SQLException e) { // TODO Auto-generated catch block e.printStackTrace(); } } }
    • Those. It turns out I should call the close method two times? dataSource.closeConnection (statement, rs) dataSource.closeConnection (connection) I also read that when closing a Statement, all open ResultSet objects associated with it are automatically closed. Those. enough to close the statment? - A.Grin
    • @ A.Grin yes. In principle, if you are sure that you always have to close the connection after executing select, you can insert a closing Statement and ResultSet into your close. I do not know whether it is necessary to close ResultSet after closing a Statement, but I don’t think that closing it if it isn’t harming will bring harm - plesser

    Yes, it is possible to avoid duplication of the code, and in different ways. Judging by the syntax of your examples, Java version 8 or higher is used. In Java 8 , features of a functional programming approach were introduced that are perfectly suited for solving your problem. Here is an example:

    1. Create a functional interface

       @FunctionalInterface public interface ConsumerWithException<T> { void accept(T t) throws SQLException; }

      You could use a ready-made java.util.function.Consumer , but the ps.setString declared a thrown SQLException exception, and java.util.function.Consumer does not declare any exceptions in the accept method, so you have to create your own interface.

    2. Create a method that performs common to all 3 variants of the problem and uses a ConsumerWithException to handle the specific code of these variants.

       public List<Employee> findEmployees(String sql, ConsumerWithException<PreparedStatement> consumer) { PreparedStatement statement = null; ResultSet rs = null; try ( Connection connection = dataSource.getConnection() ) { statement = connection.prepareStatement(sql); consumer.accept(statement);//здесь выполняются специфиеческие действия rs = statement.executeQuery(); List<Employee> list = new ArrayList<>(); while (rs.next()) { Employee tempEmployee = convertRowToEmployee(rs); list.add(tempEmployee); } return list; } catch (SQLException e) { throw new RuntimeException(e); } finally { try { if (rs != null && !rs.isClosed()) { rs.close(); } } catch (SQLException ignored) { } try { if (statement != null && !statement.isClosed()) { statement.close(); } } catch (SQLException ignored) { } } }

      Here I operate with the methods of the standard javax.sql.DataSource , in which there is no closeConnection method. It is possible that you too can do only with standard DataSource methods, since modern implementations of data sources usually have all the necessary functionality and do not require expansion.

    3. Calling 3 variants of your methods

       //вместо метода findAll() List<Employee> all = findEmployees(SQL_FIND_ALL, (PreparedStatement ps) -> {}); //вместо метода findByName(String name) String name = "John"; List<Employee> byName = findEmployees(SQL_FIND_BY_NAME, (PreparedStatement ps) -> { ps.setString(1, name); }); //вместо метода findAllAbsenceEmployeeByDate(Date startDate, Date endDate) Date startDate = Date.valueOf("2017-01-01"); Date endDate = Date.valueOf("2017-02-01"); List<Employee> absense = findEmployees(SQL_FIND_ALL_ABSENCE_BY_DATE, (PreparedStatement ps) -> { ps.setDate(1, startDate); ps.setDate(2, endDate); });