How do I simplify this integer validation?

Question

I'm new to Java, and I'm working on a method in my program that checks the users input to be within bounds, not a null value (zero), not a letter, and a positive number. So originally I incorporated two while loops within this method to check for the validity of these inputs, but I would like to simplify it in one loop. I'm getting an error when I input a letter (ex. a) after a few inputs, and I believe it is due to the two different while loops making it more complicated. Can someone help me with this please?

public static void valid(String s, int max) 
{ 
    while(sc.hasNextInt() == false) {
        System.out.println("That is not correct. Try again:");
        sc.nextLine();  

    }

    int value;
    while((value= sc.nextInt()) > max || (value= sc.nextInt()) <= 0){
        System.out.println("That is not correct. Try again: ");
        sc.nextLine();
    }
    sc.nextLine();
    return;

}

What error? Do you get *"That is not correct. Try again:"* or some exception? — RaminS, Apr 25 '16 at 16:15
If you are using `nextInt`, you will never be able to read in a letter. — OneCricketeer, Apr 25 '16 at 16:16
you also need to be using '==' in your second while loop. (value = sc.nextInt()) is not the same thing as (value == sc.nextInt()). — snerd, Apr 25 '16 at 16:21
No, he does only use one equal in the second loop. He is assigning the value of `sc.nextInt()` to `value` and then checking to see if it is greater than max. However, the second part of the while loop is the issue. It should just say `|| value <= 0` — Rabbit Guy, Apr 25 '16 at 16:25
@blahfunk I tried this and this error showed: "Syntax error, insert ") Statement" to complete WhileStatement" — O Fakos, Apr 25 '16 at 16:35
@Gendarme currently it is skipping two inputs, for ex: it asks to enter an input, so i enter 1, it gives no response, so i enter 1 again, and then the program takes it — O Fakos, Apr 25 '16 at 16:36
@OFakos That is addressed in the first part of my answer: http://stackoverflow.com/a/36846116/5221346 — RaminS, Apr 25 '16 at 16:37

score 1 · Answer 1 · answered Apr 25 '16 at 16:25

Try something more like (pseudo code):

while valid input not yet received:
    if input is an integer:
        get integer
        if in range:
            set valid input received
    skip rest of line

extended validation

With a little thought, you should be able use one "print error message" statement. But using two could be arguably better; it can tell the user what they did wrong.

RaminS · Accepted Answer · 2016-04-25T16:39:05.957

You have:

int value;
    while((value= sc.nextInt()) > max || (value= sc.nextInt()) <= 0){
        System.out.println("That is not correct. Try again: ");
        sc.nextLine();
    }

Which is doing sc.nextInt() twice, so value does not necessarily have the same value in these two cases and it is also asking you for a number twice.

A fix would be something like this:

int value;
    while((value = sc.nextInt()) > max || value <= 0) {
        System.out.println("That is not correct. Try again: ");
        sc.nextLine();
    }

which would make it better but you still have issues. If value is bigger than max, then the loop will iterate again calling nextInt() but this time you have not checked for hasNextInt(). This is why you'd better have everything in one loop. Something like this:

public static void valid(String s, int max) { 
    while(true) {
        if(!sc.hasNextInt()) { //this is the same as sc.hasNextInt() == false
            System.out.println("That is not correct. Try again:");
            sc.nextLine();
            continue; //restart the loop again
        } else {
            int value = sc.nextInt();
            if(value > max || value <= 0) {
                System.out.println("That is not correct. Try again:");
                sc.nextLine();
                continue; //restart the loop from the top - important!
            } else {
                extendedValidation(value, s);
                return;
            }
        }
    }
}

score 0 · Answer 3 · edited May 23 '17 at 11:59

What is the purpose of the String s parameter? Should you be checking that instead of a Scanner input?

Also, don't be surprised by mixing nextInt() and nextLine(). -- Source

I prefer using do-while loops for input before validation.

public static void main(String[] args) {
    Scanner input = new Scanner(System.in);
    int max = 1000;
    int val = -1;
    String in;

    do {
        // Read a string
        System.out.print("Enter a number: ");
        in = input.nextLine();

        // check for a number
        try {
            val = Integer.parseInt(in);
        } catch (NumberFormatException ex) {
            // ex.printStackTrace();
            System.out.println("That is not correct. Try again.");
            continue;
        }

        // check your bounds
        if (val <= 0 || val > max) {
            System.out.println("That is not correct. Try again.");
            continue;
        } else {
            break; // exit loop when valid input
        }

    } while (true);

    System.out.println("You entered " + val);
    // extendedValidation(value, in);
}

ManoDestra · Answer 4 · 2016-04-25T16:46:12.140

I would say that this is a lot closer to what you're looking for, in simple terms...

import java.util.Scanner;

public class Test {
    public static void main(String[] args) {
        final int MIN = 0;
        final int MAX = 10;
        Scanner sc = new Scanner(System.in);
        int value = -1;
        boolean valid;

        do {
            valid = sc.hasNextInt();
            if (valid) {
                value = sc.nextInt();
                valid = value > MIN && value < MAX;
            }

            if (!valid) {
                System.out.println("Invalid!");
                sc.nextLine();
            }
        } while (!valid);

        System.out.println("Valid Value: " + value);
    }
}

You should be able to abstract this code to suit your requirements.

How do I simplify this integer validation?

4 Answers4

Linked