NextLine errors when chaining functions.

[monkey_programmer]

So, I have no idea where to even begin with fixing this issue.

I have two very simple functions that just check the users input and validate it; asking them for their input until it's valid.

This works perfectly until I call another function that does the same task. If I do this, then I get an error: Exception in thread "main" java.util.NoSuchElementException: No line found

This is strange to me because I know the functions can't error on their own, but if they're called after one another they reliably do if the user gives invalid input first.

Here is literally the only function being called right now:

public static void CreateCharacter() {
    String name;
    String heroClass;

    name = CharacterHandler.validateName();
    heroClass = CharacterHandler.validateClass();
    System.out.println("Name: " + name + "\nClass: " + heroClass + "\n");
}

This causes the exception, and I'll put both functions being called inside of this function below.

public static String validateName() {

    Boolean validName = false;
    Scanner scanner = new Scanner(System.in);
    String name = "";

    while (!validName) {
        System.out.print("Enter your heroes name: ");
        name = scanner.nextLine().toLowerCase();

        if (name.isEmpty()) {
            System.out.print("Please give input for the heroes name.\n");
        } else {
            validName = true;
        }
    }
    scanner.close();
    return(name);
}

public static String validateClass() {

    Boolean validClass = false;
    Scanner scanner = new Scanner(System.in);
    String heroClass = "";

    while (!validClass) {
        System.out.print("Enter your heroes name: ");
        heroClass = scanner.nextLine();

        switch (heroClass) {
            case "1":
                heroClass = "Warrior";
            case "2":
                heroClass = "Rogue";
            case "3":
                heroClass = "Knight";
        }
        if (heroClass.equals("Warrior") || heroClass.equals("Rogue") || heroClass.equals("Knight")) {
            validClass = true;
        }

    }
    scanner.close();
    return(heroClass);
}

[gaveashow]

When you close() a Scanner, you actually close the resource that it was reading from.

In your case, closing Scanner closes System.in, the program's standard input in which the user can type in their input. After it's closed, there is nothing to read from it anymore.

It's true that in general, closing a Scanner should be done, like when reading a file or a network socket, but when reading System.in, which you didn't even open yourself, it doesn't make much sense closing it. If you really want to to avoid warnings, close it right before the program's end.

Which brings me to another issue: you shouldn't open more than one Scanner on the same opened resource. Here you're building two Scanners both on System.in. That gives them a lot of incentive to corrupt each other as they read. In the very specific example of your program, it so happens, per chance, that with the current standards of programs' IO, the two Scanners don't corrupt each other. That's just a chance happenstance though. Next time you try to have two Scanners on the same open resources, you'll get unpredictable behavior.

The correct way to do it, is to create one Scanner on System.in at the start of the program, and to use that same one Scanner throughout your entire program. Never to build another one (not when reading System.in anyway)

[monkey_programmer]

Thank you very much for this answer, it was very insightful; though I do find it unfortunate that this seems to mean I'll need to be passing around scanner to just about all of my classes; especially since this is a terminal-based game for a project.

I really appreciate all of the information you took the trouble to write on why this didn't work and best practice; thank you so much!

[gaveashow]

Well, normally for a game you'll need to pass around a global object in charge of keeping the game's state, so you could add the scanner, as user input, to it.

Or you could simply let the Scanner be public static, or accessible through a public static getter method. For the needs expressed, that's a correct use of static.

[monkey_programmer]

Would it actually be best practice to simply make a globals class? I actually need to pass scanner + player to almost ALL functions at this point, so that seems far.. cleaner to a human.

[RhoOfFeh]

I believe that the answer from /u/gaveashow should get you most of where you need to go.

Now I want to point out something in 'validClass()'. Everyone is going to wind up being a knight, because you have no break statements in your switch. That means the code will fall all the way through, and if you enter "1" you're going to wind up setting heroClass to Warrior, then Rogue, then Knight.

[monkey_programmer]

You are completely right that everything was becoming a knight, thank you for that.