5
\$\begingroup\$

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();
            }
        }
    }
}
```
\$\endgroup\$
1
\$\begingroup\$

You can decide between to structural design patterns:

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 👀)
\$\endgroup\$
  • \$\begingroup\$ Thank you, i was looking for a solution like this. But one thing i have noticed: What if i want to set an additional parameter for the PreparedStatement? Lets say i want to put an extra E-Mail textfield on the registration form, then i would have to add something like this statement.setString(3, credential.getEmail()); in the execute method. This would result in false queries for those who only accepts two parameters. I hope im wrong because this is a cool solution, but to me this only seems to work as long as query stays identical. \$\endgroup\$ – Clayy91 Aug 17 at 19:52
  • \$\begingroup\$ Maybe you can create an abstract class Query and all your concrete query types that can interact with a Credential. \$\endgroup\$ – Roman Aug 18 at 20:46
5
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ thanks for your answer! Few things ive noticed when i tried to implement your solution: The executeUpdate() Method returns an Integer, so i cant have ResultSet as return type. I could however, use executeQuery() for all SQL-querys. Problem is, i would have to catch a SQLException in the checkLogin Method, because result.next() throws such. Kind of destroys the intend of the query Method. I thought about some kind of Interface/Abstract Class Pattern in the first place to solve this problem, but cant figure out. I will continue playing around with your suggestion tho! \$\endgroup\$ – Clayy91 Aug 16 at 20:29
  • \$\begingroup\$ @Clayy91 You could implement a QueryResult class with a resultType property that based on it you'll know what other property of that class contains the result (int resultInt or ResultSet resultSet for example) - or look straight at the property you expect to contain the result according to the type of query you made. \$\endgroup\$ – potato Aug 16 at 22:28
4
\$\begingroup\$

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.

\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.