Ttdfjt

I'm going through some code and trying to figure out how to get rid of duplicates from the following two methods. Both methods execute a simple login/register query to a local MySQL DB. I couldnt figure out a good design pattern to solve my problem, especially to get rid of the duplicate try-catch.

Any advice?

public class DatabaseHandler 

 [...]

 public static boolean checkLogin(String username, String password) 
 Connection connection = createConenction();
 PreparedStatement statement = null;

 String query = "select * from users where username = ? and password = ? ";

 try 
 statement = connection.prepareStatement(query);
 statement.setString(1, username);
 statement.setString(2, password);
 ResultSet result = statement.executeQuery();
 return result.next(); //True if User exists
 catch(SQLException e) 
 return false;
 finally 
 try 
 statement.close();
 catch (SQLException e) 
 e.printStackTrace();
 
 
 

 public static boolean registerUser(String username, String password) 
 Connection connection = createConenction();
 PreparedStatement statement = null;

 String query = "insert into users (username, password) values (?, ?)";

 try 
 statement = connection.prepareStatement(query);
 statement.setString(1, username);
 statement.setString(2, password);
 statement.executeUpdate();
 return true;
 catch(SQLException e) 
 return false;
 finally 
 try 
 statement.close();
 catch (SQLException e) 
 e.printStackTrace();
 
 
 

```

You can decide between to structural design patterns:

• Template Method Pattern


• Strategy Pattern

I would go with the Template Method Pattern because you do not reuse the algorithm which is a benefit of the Strategy Pattern.

In the first step we need to create an abstract class which has all the common code and a invocation of an abstract method where the algorithms distinguish.

abstract class PersistentAction 

 private final Connection connection;
 private final String username;
 private final String password;

 PersistentAction(Connection connection, String username, String password) /* ... */ 

 final boolean execute(String query) 
 try(PreparedStatement statement = connection.prepareStatement(query)) 
 statement.setString(1, credential.getUsername());
 statement.setString(2, credential.getPassword());
 ResultSet result = statement.executeQuery();
 return evaluate(result);
 catch(SQLException e) 
 return false;
 
 

 abstract boolean evaluate(Result result);

After that we can create our algorithms which extends from our abstract class and implement the abstract method.

class HasNextEvaluation extends PersistentAction 

 @Override
 protected boolean evaluate(ResultSet result) 
 return result.next();
 



class ConstantTrue extends PersistentAction 

 @Override
 protected boolean evaluate(ResultSet result) 
 return true;
 

After the two steps we can achieve the following:

public class DatabaseHandler 

 private static final Connection connection = createConenction();

 public static boolean checkLogin(String username, String password) 
 String query = "select * from users where username = ? and password = ? ";

 PersistentAction action = new HasNextEvaluation(connection, username, password);
 return action.execute(query);
 

 public static boolean registerUser(String username, String password) 
 String query = "select * from users where username = ? and password = ? ";

 PersistentAction action = new ConstantTrue(connection, username, password);
 return action.execute(query);
 

From here I would improve the parameter list checkLogin(String username, String password) by using an Paramter Object.

public static boolean checkLogin(Credential credential) 
 /* ... */

There are two benefits that are in my mind. The first is the short parameter list and the second one is that you could have multiple types of credentials:

• username and password


• email and password


• mobile number


• (even with biometrics 👀)

Funny coincidence, I just created the solution for your problem (but in PHP). The common thing here is the one-time use of a prepared statement, the difference is in the query string and arguments, so what you need is a function that accepts a query string, an argument determining what execution function to use (executeQuery() or executeUpdate()) and the parameters for the query:

// note: QueryType is an enum you should define
public static ResultSet query(String sql, QueryType queryType, Object... queryArgs)
 // ...

Include the try-catch blocks in this function too of course, and return null when according to queryType there should be no results.

Note: I have no experience with database interaction with Java, so if there are only 2 different statement execution functions then you can use a boolean argument instead of the enum QueryType.

Then your code could look like this:

public static boolean checkLogin(String username, String password) 
 String sql = "select * from users where username = ? and password = ? ";
 ResultSet result = query(sql, QueryType.RETURN_RESULT, username, password);
 return result.next(); // true if User exists

A note about your query string: I find it more convenient to use CAPS for keywords and lowercase text for names, it highlights the structure of the query.

You must never store passwords as plain text in the database. Read an article about password hashing to avoid this mistake in the future.

Make sure that you have a unique index on the username column. Otherwise it will be possible to create several users with the same username, and with equal or differing passwords.

You should really try to use a more modern approach to closeable resources. The try-with-resources statement has been introduced in Java 7 (2011).

This gets rid of all the hocus-pocus around closing and nested exceptions for you. Trivial rewrite of the first function:

public static boolean checkLogin(String username, String password) 
 Connection connection = createConenction();
 String query = "select * from users where username = ? and password = ? ";

 try (
 PreparedStatement statement = connection.prepareStatement(query)
 ) 
 statement.setString(1, username);
 statement.setString(2, password);
 ResultSet result = statement.executeQuery();
 return result.next(); //True if User exists
 catch(SQLException e) 
 e.printStackTrace();
 return false;
 

Furthermore, if the method name createConnection() is not a lie, you should also close the connection. (Otherwise rename it to getConnection()):

public static boolean checkLogin(String username, String password) 
 String query = "select * from users where username = ? and password = ? ";

 try (
 Connection connection = createConenction();
 PreparedStatement statement = connection.prepareStatement(query)
 ) 
 statement.setString(1, username);
 statement.setString(2, password);
 ResultSet result = statement.executeQuery();
 return result.next(); //True if User exists
 catch(SQLException e) 
 e.printStackTrace();
 return false;
 

Apart from that little optimization in syntax, I do not think that the methods bear enough similarity to warrant a common abstraction. Writing and retrieving a line of data is a totally different business case and should be kept separate to be able to develop into different directions in the future.

If you do not want to do all that basic handling of statements, result sets, rows, columns, rather look for a well established OR-mapping framework (e.g. JPA, Hibernate) instead of rolling your own.