tags:

views:

78

answers:

2

Hi, I have a Manager class that saves data in the SQL table and also get result from SQL table and test these data.when I run my program,one frame will be shown that gets ID and password and if they are correct ,the other frame will be shown.BUT I don't know that why it just test the last row of SQL table?? i mean if I set those text fields with the other IDs and passwords except from last row.it will show the data are wrong(I have set it before for wrong data)

Manager class:

 public static boolean Test(String userName, String password) {
    boolean bool = false;
    Statement stmt = null;
    try {
        stmt = conn.createStatement();

        ResultSet rst = null;

        rst = stmt.executeQuery("SELECT yahooId , password FROM clienttable");


        while (rst.next()) {
            if (rst.getString(1).equalsIgnoreCase(userName) && rst.getString(2).equalsIgnoreCase(password)) {
                bool = true;
            } else {
                bool = false;
            }
        }
    } catch (SQLException ex) {
        Logger.getLogger(Manager.class.getName()).log(Level.SEVERE, null, ex);
    }
    return bool;



}

my button's perform action in the frame which get the ID and password and test it:

 private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {                                         
    try {
        submit();
    } catch (ConnectException ex) {
        JOptionPane.showMessageDialog(this, "You coudn't connect to the server successfully,try it again", "Sign_In Problem", JOptionPane.OK_OPTION);

    }
    clear();

} 

  private void submit() throws ConnectException {

    String id = idField.getText();
    char[] pass1 = passField.getPassword();
    String pass = new String(pass1);
    if (id.equals("") || pass.equals("")) {
        Toolkit.getDefaultToolkit().beep();
        JOptionPane.showMessageDialog(this, "You should enter an ID and password", "Sign_In Problem", JOptionPane.OK_OPTION);
        return;
    } else {
        boolean b = Manager.Test(id, pass);
        client.setCurrentName(id);
        if (b == true) {
            this.setVisible(false);



            ListFrame frame = new ListFrame(client);
            frame.setVisible(true);





        } else {

            JOptionPane.showMessageDialog(this, "You have entered wrong datas,try it again", "Sign_In Problem", JOptionPane.OK_OPTION);
            return;
        }
    }
}

EDIT:

I have edited my manager class(test method) but still it works like past!!

  public static boolean Test(String userName, String password) {
    boolean bool = false;
    PreparedStatement stmt = null;
    ResultSet resultSet = null;
    try {
        stmt = conn.prepareStatement("SELECT id FROM clienttable WHERE yahooId = ? AND password = ?");
        stmt.setString(1, userName);
        stmt.setString(2, password);
        resultSet = stmt.executeQuery();
        bool = resultSet.next();

    } catch (SQLException ex) {
        Logger.getLogger(Manager.class.getName()).log(Level.SEVERE, null, ex);
    } finally {
        try {
            resultSet.close();
            stmt.close();
            conn.close();
        } catch (SQLException ex) {
            Logger.getLogger(Manager.class.getName()).log(Level.SEVERE, null, ex);
        }

    }

    return bool;



}
+7  A: 

Because you're hauling the entire database table down into Java's memory and testing every row in a while loop. You don't break the loop if a match is found so that it continues overwriting the boolean outcome until with the last row.

That said, you really don't want to do the comparison in Java. Just make use of the SQL WHERE clause. That's much more efficient and really the task a DB is supposed to do. Don't try to take over the DB's work in Java, it's only going to be inefficient.

public boolean exists(String username, String password) throws SQLException {
    Connection connection = null;
    PreparedStatement preparedStatement = null;
    ResultSet resultSet = null;
    boolean exists = false;

    try {
        connection = database.getConnection();
        preparedStatement = connection.prepareStatement("SELECT id FROM client WHERE username = ? AND password = ?");
        preparedStatement.setString(1, username);
        preparedStatement.setString(2, password);
        resultSet = preparedStatement.executeQuery();
        exists = resultSet.next();
    } finally {
        close(resultSet);
        close(preparedStatement);
        close(connection);
    }

    return exists;
}

You see that I made a few enhancements:

  1. Use preparedstatement.
  2. Do not use equalsignorecase. A password of "FooBar" should NOT be the same as "foobar".
  3. Gently acquire and close resources in same scope to avoid leaking.
  4. Have it in an independent and reuseable non-static DAO method.

To learn more about using JDBC the proper way you may find this basic tutorial useful.

BalusC
i have done it.(I edited my post) but it doesn't work correctly!!
Johanna
think we might need a bit more information than it doesn't work correctly... @BalusC's answer is about as good as it's going to get with the information you have provided
Gareth Davis
@Johanna: Then the problem lies somewhere else. Debug it. If you mean that `false` is been returned, then it simply means that there's no match in the DB :) By the way, your edited code is in JDBC perspective still wrong. You're still leaking the `Connection` and not closing the resources each in its own try-catch (and worse, the whole method is still `static`). Your app might break sooner or later when used extensively in production.
BalusC
@Johanna: if you want more assistance, you really, *really* need to learn to ask questions the smart way. Plain saying "it doesn't work!" gives us really **nothing** to work with. Elaborate in detail what exactly happens and also elaborate in detail what exactly happens **not** (i.e. your expectations). With in detail, I mean at code (developer) level and not at end-user level.
BalusC
+1 from me - a typically well written response.
duffymo
nice answer,thanks a lot (sorry for late thanks!)
Johanna
+2  A: 

The simple answer is that your while loop should terminate when you find a match, so it should read:

    while (rst.next() && bool == false) {
        if (rst.getString(1).equalsIgnoreCase(userName) && rst.getString(2).equalsIgnoreCase(password)) {
            bool = true;
        }
    }

Note that it would be more efficient to select only rows that match your user id & password, something along the lines of the following: (note that error handling is left as an exercise for the reader)

PreparedStatement stmt; stmt = conn.prepareStatement("SELECT yahooId , password FROM clienttable WHERE yahooId = ?"); stmt.setString(1, "userName");
ResultSet rst = null; rst = stmt.executeQuery();

if (rs != null && rs.getString("password").equals(password)) { bool = true; }

Angelo Genovese