Java skips else condition

Question

Hello

I'm not sure why, but for some reason, the following code skips the else condition. I've tried just about everything I can think of, including switching the code blocks, but it still skips the else part. Basically, I want this method to return String temp = "no" if String docID that is passed to the method is not found in the FILESTATUS database, and String temp = "yes" if it is found.

static String checkDocID(String docID)
{
    String temp = null;

    System.out.println("Checking if data already exists in database...");
    try
    {
        Main.stmt = Main.con.createStatement();
        String command = "SELECT * FROM FILESTATUS WHERE ID='" + docID + "'";
        ResultSet queryResult = Main.stmt.executeQuery(command);
        if (!queryResult.next())
        {
            temp = "no";
        }
        else
        {
            while (queryResult.next())
            {
                String result = queryResult.getString("ID");
                if (result.equals(docID))
                {
                    temp = "yes";
                    break;
                }
            }
        }
        Main.stmt.close();
    }
    catch (Exception ex) {ex.printStackTrace();}
    return temp;
}

Thanks!

Answer 1

Might be better to restructure this loop as:

static String checkDocId(String docId) {
    String temp = "no";

    while (queryResult.next()) {                
        String result = queryResult.getString("ID");

        if (result.equals(docID)) {
            temp = "yes";
            break;
        }
    }

    return temp;
}

Some people don't like using break (I usually don't) so you can use a boolean in your while (I find that it reads more like english and you can tell the terminating condition right from the while instead of looking for an if inside):

static String checkDocId(String docId) {
    boolean found = false;

    while (queryResult.next() && !found) {
        String result = queryResult.getString("ID");
        found = result.equals(docID);
    }

    return found ? "yes" : "no";
}

You're performing a needless comparison otherwise. Remember, a while is just an if with a goto at the end ;)

As far as your problem is concerned, what Paul said is correct. Eitherway, I would still restructure the loop so that it's more elegant.

Answer 2

Because you end up calling queryResult.next() in the "if" and again in the while, you're skipping the first result. If there is only one result, the while loop will never execute.

If I can make a couple of suggestions:

• Use bound variables in a PreparedStatement rather than putting "docID" in the query string
• Don't test result.equals(docID) since the query already assured that.
• prefer boolean to String "yes" or "no"
• set the result to "no" or false, then set it to "yes" or true in the loop. The extra assignment is probably faster than the extra test, plus you can skip the do{}while which most people find harder to read.

Answer 3

rs.next() rolling current cursor after each call

try with

static String checkDocID(String docID)
{
    String temp = null;

    System.out.println("Checking if data already exists in database...");
    try
    {
        Main.stmt = Main.con.createStatement();
        String command = "SELECT * FROM FILESTATUS WHERE ID='" + docID + "'";
        ResultSet queryResult = Main.stmt.executeQuery(command);
        boolean found = queryResult.next();
        if (!found)
        {
            temp = "no";
        }
        else
        {
            do {
                String result = queryResult.getString("ID");
                if (result.equals(docID))
                {
                    temp = "yes";
                    break;
                }
            } while (queryResult.next())
        }
        Main.stmt.close();
    }
    catch (Exception ex) {ex.printStackTrace();}
    return temp;
}

Answer 4

You are calling queryResult.next twice before you are trying to read it. To prove this, put a println or something right after the else (before the while).

Since you are selecting by a particular ID, moving to next() twice will in fact fail the second time (presumably) and never execute the while block.

Answer 5

After a bit astonishment about the code (leaking DB resources, not trusting the DB that it returns the right docID, doing a SELECT * while you don't need all of the columns at all, not taking benefit of SQL-injection-sanitization powers of PreparedStatement, using String instead of boolean to denote a true/false value, a messy code flow), here's how it should really be done instead:

private static final String SQL_EXIST_DOCID = 
    "SELECT id FROM filestatus WHERE id = ?";

public boolean exist(String docID) throws SQLException {
    Connection connection = null;
    PreparedStatement statement = null;
    ResultSet resultSet = null;
    boolean exist = false;

    try {
        connection = database.getConnection();
        statement = connection.prepareStatement(SQL_EXIST_DOCID);
        statement.setString(1, docID); // Shouldn't it be a Long instead? Characterbased PK's are *far* from efficient.
        resultSet = statement.executeQuery();
        exist = resultSet.next();
    } finally {
        if (resultSet != null) try { resultSet.close(); } catch (SQLException logOrIgnore) {}
        if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
        if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
    }

    return exist;
}

Clear, concise and simple as that. For more insights about using basic JDBC the right way you may find this article useful. I hope that you take it serious to learn from it. Really.