Java else statement error

Question

The JOptionPane box keeps appearing at least 3 times if I input the incorrect username and password. Firstly, is it even a while loop.

I spent hours looking at the code and trying to modify it but no luck. Couldn't get much help either, but I would greatly appreciate it if someone could point out or fix my error.

The code where I am getting problems is below.

       try{
        PreparedStatement ps = con.prepareStatement("SELECT Login, Password, Restriction FROM Users");
        ResultSet rs = ps.executeQuery();
        while(rs.next()){
            if (username.equals(rs.getString("Login")) && password.equals(rs.getString("Password")) && ("Admin".equals(rs.getString("Restriction")))){
                this.setVisible(false);
                MainMenuAdmin admin = new MainMenuAdmin();
                admin.setVisible(true);
            }
            else if (username.equals(rs.getString("Login")) && password.equals(rs.getString("Password")) && ("Engineer".equals(rs.getString("Restriction")))){
                this.setVisible(false);
                MainMenuEngineer engineer = new MainMenuEngineer();
                engineer.setVisible(true);
            }
            else if (!username.equals(rs.getString("Login")) || !password.equals(rs.getString("Password"))){
              JOptionPane.showMessageDialog(null, "Login Failed");
            }
        }  
    }
    catch(SQLException sqle){
        sqle.printStackTrace();
    }

Also how do code it such that if a username or password doesn't exist, it says 'Username or Password doesn't exist'?

Probably there are three users. That's why the pane appears multiple times. This is, because you're iterating over every user in the database and update the GUI on every decisision. You could set flags instead and update the GUI after the loop. Or you completely change the approach: lookup the entered user/password/restriction using your prepared statement (you have to take care of "SQL injection"!) and determine the restriction from the (one) result (if any). — try-catch-finally
– try-catch-finally, Commented Mar 15, 2014 at 10:07
The problem is not with the else statement but with your logic: 1. you run the code for all results; 2. you don't even use the query to filter out entries that should not match; 3. you open a database cursor but never close it — fge
– fge, Commented Mar 15, 2014 at 10:07

Keerthivasan · Accepted Answer · 2014-03-15 10:09:32Z

1

Your approach is very much flawed. There is no point in fixing the error in a bad approach. Please change your query like this

SELECT Restriction,ADD_REQD_FIELDS..., FROM Users Login = ? and Password = ?

If the resultset count is 1, then you have a valid user.This way you can check the authentication of the user and retrieve the user information from the database as well. Your approach is unnecessarily over complicated.

answered Mar 15, 2014 at 10:09

Keerthivasan

12.9k2 gold badges34 silver badges55 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Boris the Spider · Accepted Answer · 2014-03-15 10:14:56Z

1

You approach is completely wrong.

You should actually query the data. Use the database that you have.
You should close() your resources when are done with them.

This is a method to check the login details. It queries the username and password from the database.

public String checkLogin(final Connection con, final String username, final String password) throws SQLException {
    final String q = "SELECT Restriction FROM Users WHERE Login = ? AND Password = ?";
    try (final PreparedStatement ps = con.prepareStatement(q)) {
        ps.setString(1, username);
        ps.setString(2, password);
        try (final ResultSet rs = ps.executeQuery()) {
            if (!rs.next()) {
                return "Invalid";
            }
            return rs.getString("Restriction");
        }
    }
}

You would use it thus:

public void checkLogin() {
    try {
        final String loginResult = checkLogin(con, username, password);
        switch (loginResult) {
            case "Admin":
                this.setVisible(false);
                MainMenuAdmin admin = new MainMenuAdmin();
                admin.setVisible(true);
                return;
            case "Engineer":
                this.setVisible(false);
                MainMenuEngineer engineer = new MainMenuEngineer();
                engineer.setVisible(true);
            default:
                JOptionPane.showMessageDialog(null, "Login Failed");
        }
    } catch (SQLException sqle) {
        sqle.printStackTrace();
    }
}

answered Mar 15, 2014 at 10:14

Boris the Spider

61.4k6 gold badges111 silver badges177 bronze badges

6 Comments

user3138212 Over a year ago

Thank you so much. I tried this, however, in the checkLogin method, it told me to use catch and as a result, I have not been able to return the 'return rs.getString("Restriction")'. I have had to return null instead. The catch ruined it all! How can I fix this. You are a life saver and legend!!!

user3138212 Over a year ago

I am using Java 7. The catch clause ruined the code lol. My program doesn't compile without the catch clause. How do I fix this?

Boris the Spider Over a year ago

The code then does compile without the catch; are you missing the throws clause on your method?

user3138212 Over a year ago

Oh yeah, I missed the throws clause, whoops. I am getting a new error, 'No operations allowed after connection closed'.

Boris the Spider Over a year ago

@user3138212 the error is fairly clear. Your connection is closed.

|

Collectives™ on Stack Overflow

Java else statement error

2 Answers 2

Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related