Validation of integer inputs from a BufferedReader

Question

At my school, we are using the BufferedReader for keyboard input for our console apps and I don't want to have to write ways to validate input for every project so I am writing a class to validate user input. I have only written the validation methods for ints and want feedback before I do so for doubles, etc.

I want to make the class dynamic and allow the user to use their own BufferedReader if they want to and specify a range if they want.

package ValidateInput;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class ValidateInput {
    public static int getInt() {
        BufferedReader in = new BufferedReader(new 
        InputStreamReader(System.in));
        String errorMessage = "Please enter a valid integer: ";
        boolean invalid = true;
        int output = 0;

    do {
        try {
            //Get input from keyboard buffer and attempt to parse to an int.
            output = Integer.parseInt(in.readLine());
            //Only executes boolean assignment if an exception isn't thrown.
            invalid = false;
        } catch(NumberFormatException e) {
            //If it is not a valid int print error message.
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of validateInt method.

public static int getInt(BufferedReader in) {
    String errorMessage = "Please enter a valid integer: ";
    boolean invalid = true;
    int output = 0;

    do {
        try {
            output = Integer.parseInt(in.readLine());
            invalid = false;
        } catch(NumberFormatException e) {
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of validateInt method.

public static int getInt(int value) {
    BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
    String errorMessage = "Please enter the integer value " + value + ": ";
    boolean invalid = true;
    int output = 0;

    do {
        try {
            output = Integer.parseInt(in.readLine());
            //If the input is a valid int, check if it matches the value specified.
            if(output != value) {
                System.out.print(errorMessage);
            } else {
                invalid = false;
            }
        } catch(NumberFormatException e) {
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of getInt method.

public static int getInt(int value, BufferedReader in) {
    String errorMessage = "Please enter the integer value " + value + ": ";
    boolean invalid = true;
    int output = 0;

    do {
        try {
            output = Integer.parseInt(in.readLine());
            if(output != value) {
                System.out.print(errorMessage);
            } else {
                invalid = false;
            }
        } catch(NumberFormatException e) {
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of getInt method.

public static int getInt(int startRange, int endRange) {
    //If the startRange is greater than the end range swap the values.
    if(startRange > endRange) {
        int temp = endRange;
        endRange = startRange;
        startRange = temp;
    }

    BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
    String errorMessage = "Please enter a valid integer between " + startRange + " and " + endRange +": ";
    boolean invalid = true;
    int output = 0;

    do {
        try {
            output = Integer.parseInt(in.readLine());
            //If the input is a valid int, check to see if it is in range.
            if(output < startRange || output > endRange) {
                System.out.print(errorMessage);
            } else {
                invalid = false;
            }
        } catch(NumberFormatException e) {
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of validateInt method.

public static int getInt(int startRange, int endRange, BufferedReader in) {
    if(startRange > endRange) {
        int temp = endRange;
        endRange = startRange;
        startRange = temp;
    }

    String errorMessage = "Please enter a valid integer between " + startRange + " and " + endRange +": ";
    boolean invalid = true;
    int output = 0;

    do {
        try {
            output = Integer.parseInt(in.readLine());
            if(output < startRange || output > endRange) {
                System.out.print(errorMessage);
            } else {
                invalid = false;
            }
        } catch(NumberFormatException e) {
            System.out.print(errorMessage);
        } catch(IOException e) {
            e.printStackTrace();
        }
    } while(invalid);

    return output;
}//End of validateInt method.
}//End of class.

Answer 1

Thanks for sharing your code.

duplicated code

IMHO the main problem is this code is duplication.

In every method you repeat the same do/while block. In some of the methods there is a nested if/else block but this could also be located after the do/while block without changing the logic.

---

If we want to resolve this code duplication we need to apply the Single Responsibility Pattern and the Separation of Concerns Pattern (SRP/SCP).

This two patterns state that each method should have a single clear defined task. So we first have to identify the tasks in this program:

• create the BufferedReader instance
• acquire next line
• repeat when reading of line fails
• convert the input String to an integer
• repeat when conversion to integer fails
• do additional check on the value
• repeat when the value checks fails

The aim is to put as much of this responsibilities in to separate methods as possible.

So lets start with the easy bits:

   private BufferedReader createBufferedReader(){
      return new BufferedReader(new InputStreamReader(System.in));
   }

   private String readNextLine(BufferedReader in) throws IOException{
      return in.readLine();
   }

   private int convertStringToInt(String valueString) throws NumberFormatException {
      return Integer.parseInt(valueString);
   }

Wen can bring this together in a "delegating" method which calls this new method one after the other and which handles the exceptions thrown:

private int getNumberFromUser(BufferedReader in) {
    while (true) { // we end the loop by a return, not by a condition.
        try {
            String valueString = readNextLine(in);
            // return leaves the method and therefor the loop too.
            return convertStringToInt(valueString);
            // this method will not return 
            // and the loop will not end if convertStringToInt() throws an
            // exception!
        } catch (IOException ex) {
            System.err.println("could not acquire next line from system input: " + ex.getMessage());
        } catch (NumberFormatException ex) {
            System.err.println("could not convert input string: " + ex.getMessage());
        }
    }
}

You can call the this service method from your public interface methods as needed.

public static int getInt() {
   // This is a specialized form which calls the more general form below
   BufferedReader in = createBufferedReader();
   return getNumberFromUser(in);
}

public static int getInt(BufferedReader in) {
   return getNumberFromUser(in);
}

As you can see we do not do any checks on the value (yet).

Unless you know if the concept of an interface we cannot reduce the duplication around the value checks which repeat the program unless the rule is met. But we can avoid having a copy because of the additional parameter:

public int getInt(int value) {
   // This is a specialized form which calls the more general form below
   BufferedReader in = createBufferedReader();
   return getInt(value, in);
}

public int getInt(int value, BufferedReader in) {
   while(true){ // we end the loop by a return, not by a condition. same as before.
      int value =  getNumberFromUser(in);
      if(output != value) {
            System.out.print("Please enter the integer value " + value + ": ");
        } else {
            return value;
        }
   }
}

---

public int getInt(int startRange, int endRange) {
   BufferedReader in = createBufferedReader();
   return getInt(startRange, endRange, in);
}

public int getInt(int startRange, int endRange, BufferedReader in) {
    // If the startRange is greater than the end range swap the values.
    if (startRange > endRange) {
        int temp = endRange;
        endRange = startRange;
        startRange = temp;
    }
    while (true) {
        int output = getNumberFromUser();
        if (output < startRange || output > endRange) {
            System.out.print("Please enter a valid integer between " + startRange + " and " + endRange + ": ");
        } else {
            return output;
        }
    }
}

avoid static

You might have seen that I removed all the static key words.

This implies that you create an object of the class ( new ValidateInput() ) to be able to access the Methods:

System.out.println(new ValidateInput().getInt());

When getting used to use static methods you distract yourself from using inheritance and polymorphism which are the real benefits of using an object oriented language like Java.

As a rule of thumb you should not use the static key word unless you have a very good reason to do so.

useless comments

You program has some comments but none of then us useful since it does not add any information not expressed by the code itself.

You should not add comments just for formal reasons. Such comments are useless at best but usually they turn into lies as the code develops because the first coder changing the content might forget to change the comment too and the next coder refuses to change the comment because she does not know if the comment is wrong or the code...