Check input from user

Aim: Read in the userID, check if the userID has 10-digits and check if it not exists in database, when both right, the program will leave the loop.

This code works on my project, but I think its not the most elegant solution. Have someone any improvements?

InputReader.java

public class InputReader {

    private String userID;

    public void readInput() throws SQLException {

        boolean UserIDExists = false;
        int existsValue = 0;

        try {

    BufferedReader input = new BufferedReader(new InputStreamReader(System.in));

    System.out.println("UserID (10-digits):");

    while(!UserIDExists) {

        this.userID = input.readLine();

        while (this.userID.length() != 10) {

        System.out
                .println("Input must be 10 digits. Type again: :");
        this.userID = input.readLine();

        }

    CustomerInfo customer = new CustomerInfo();
    existsValue = customer.checkCustomer(this.userID);

    if (existsValue == 1) {

        UserIDExists = false; 
        System.out.println("UserID exists already. Type again:");   

    } else {

        UserIDExists = true;

    }   
    }

CustomerInfo.java

```
public class CustomerInfo {

private Connection conn = null;
private ResultSet resultSet = null;

public CustomerInfo () {

try {
conn = null;
JDBCConnection jdbcConn = new JDBCConnection();
conn = jdbcConn.openConnection();

} catch (Exception e) {
throw e;
}
};

public int checkUserID (String userID) throws SQLException {
PreparedStatement pstmt = null;
String sql = "SELECT * FROM TABL0001 WHERE USERID = ?";
pstmt = conn.prepareStatement(sql);
pstmt.setString(1, userID);
resultSet = pstmt.executeQuery();

if (!resultSet.next()){
return 0;

}
return 1;

You are using prepared statements which is good, but why does your query return all columns ? A simple SELECT USERID FROM TABL0001 WHERE USERID = ? would be enough and if multiple users access this code, the database load would be low which is good.

The checkUserID() method is returning an int which only will be either 0 or 1 and this is screaming for a boolean which also would simplify the method. Also the name of the method is a little bit misleading. A better name would be existUserID

Like

public boolean existUserID (String userID) throws SQLException {

    String sql = "SELECT * FROM TABL0001 WHERE USERID = ?";     

    PreparedStatement pstmt = conn.prepareStatement(sql);
    pstmt.setString(1,  userID);
    resultSet = pstmt.executeQuery();

    return resultSet.next();
}

You aren't closing the database connection. Also, because you are always creating a new CustomerInfo you really should make the former checkUserID() method static and create the connection there.

If you are using Java 7 or above you should use try with ressources to ensure that for any exception that the ressources are closed.

If you are using Java prior version 7 you should use a finally block where you the close the ressources.

The name readInput does not really say much. You should always use meaningfull names for classes, methdos and parameters. Also readInput would let me think I would get something back, but no, the method is void.

boolean UserIDExists = false;

should be named using camelCase casing.

Instead of while(!userIDExists) you should just make while(true) and if the userid does not exists just break; out of the loop.

while (true) {

    this.userID = input.readLine();

    while (this.userID.length() != 10) {

        System.out
                .println("Input must be 10 digits. Type again: :");
        this.userID = input.readLine();

    }

    // assuming existUserId() is static
    if (CustomerInfo.existUserId(this.userId)) {
        break;
    }
    System.out.println("UserID exists already. Type again:");
}

by extracting the reading from the input stream to a separate method this can be simplified some more and will reduce the responsebilities of the method.

private String readUserInput(String message) {

    System.out.println(message);

    BufferedReader input = new BufferedReader(new InputStreamReader(System.in));        
    return input.readLine();
}

leading to

String message = "UserID (10-digits):";
while (true) {

    this.userID = readUserInput(message);

    while (this.userID.length() != 10) {

        this.userID = readUserInput("Input must be 10 digits. Type again: ");

    }

    // assuming existUserId() is static
    if (!CustomerInfo.existUserId(this.userId)) {
        break;
    }
    message = "UserID exists already. Type again:";
}

Based on the comment I changed the above implementation to break if the userId does not exists. But hey, the variable you formerly used UserIDExists is very very very !! misleading. One would assume (like I did) if a userId exists then the boolean variable UserIDExists would become true !

StackExchange Code Review Q#83898, answer score: 5