0

This is a code that i'm creating helping users to log in into my application a method login that gets two arguments username and password.

I want to be specified with my error messages to the user, so:

  1. if the email does not exist in the table it shows "email is not registered"
  2. if the email exists and the password is incorrect is shows " incorrect password"

Is my code efficient? I need your opinion. What do I need to improve\change?

Here's the method

    private String command;

private ResultSet resultSet;
private PreparedStatement statement;
private Connection connection;

String jdbcUrl = "jdbc:mysql://localhost:3306/registered";
String jdbcUser = "...";
String jdbcPassword = "...";

public boolean login(String eml, String pwd) {

    try {

        Class.forName("com.mysql.jdbc.Driver");
        connection = DriverManager.getConnection(jdbcUrl, jdbcUser,
                jdbcPassword);

        command = "SELECT Email FROM users WHERE Email LIKE '" + eml + "';";
        statement = connection.prepareStatement(command);
        resultSet = statement.executeQuery();

        if (!resultSet.isBeforeFirst()) {
            System.out.println("Email (" + eml + ") is not registered ! ");

            // show error message
        } else {
            command = "SELECT Email,Password FROM users WHERE Email LIKE '"
                    + eml + "' AND Password LIKE '" + pwd + "';";
            statement = connection.prepareStatement(command);
            resultSet = statement.executeQuery();

            if (!resultSet.isBeforeFirst()) {
                System.out.println("Password for Email (" + eml
                        + ") is incorrect ! ");

                // show error message
            }
            else {
                System.out.println("Logged in!");
            }

        }

    } catch (SQLException e) {
        System.out.println("SQLException: " + e.getMessage());
        System.out.println("Vendor error: " + e.getErrorCode());

    } catch (ClassNotFoundException e) {
        e.printStackTrace();
    }

    return false;
}
Luiggi Mendoza
  • 85,076
  • 16
  • 154
  • 332
amjazzed
  • 29
  • 7

1 Answers1

2

Is my code efficient?

No, it is not. The main problem in the performance of this method is that you open the connection manually here:

Class.forName("com.mysql.jdbc.Driver");
connection = DriverManager.getConnection(jdbcUrl, jdbcUser, jdbcPassword);

This is a bottleneck because you will open a physical connection to the database and close it, and this is a high-cost operation. This should be replaced for a database connection pool and retrieve the connection from there. Some options to implement a database connection pool:

More info:

What do I need to improve\change?

Apart of the way you obtain the connection, the current code is prone to SQL Injection because you concatenate the string to generate the SQL statement:

command = "SELECT Email FROM users WHERE Email LIKE '" + eml + "';";

Use PreparedStatement accordingly and set parameters:

command = "SELECT Email FROM users WHERE Email = ?";
statement = connection.prepareStatement(command);
statement.setString(1, eml);
resultSet = statement.executeQuery();

Not really a problem with your code but with your design. The password of the users should be encrypted and validated on database only. Looks like you're storing the password as plain text, which is a valid option for learning purposes, but not to use in real world applications.

Community
  • 1
  • 1
Luiggi Mendoza
  • 85,076
  • 16
  • 154
  • 332