3
\$\begingroup\$

Often end up using a Scanner to grab some simple input and often when dealing with Number input it can result in a lot of little bits just to check for valid input. In an effort to reduce this, I tried making it more generic with a Functional Interface and a self contained class to hold the Scanner. The following is the result:

import java.util.Scanner;
import java.util.function.Function;

public class ScannerInput {

  private static Scanner scanner = new Scanner(System.in);
  
  private ScannerInput() {}
  
  public static String getLine() {
    return scanner.nextLine();
  }
  
  public static <T> T getParsed(Function<String, T> function) {
    T input = null;
    while(input == null) {
      try {
        input = function.apply(scanner.nextLine());
      } catch (NumberFormatException nfe) {
        System.out.println("Incorrect Value Type. Try Again: ");
      }
    }
    return input;
  }
}

The idea is to use the parseFoo() methods from the Number child classes to tell it what to try parsing it as.

public class GenericInputTest {
  public static void main(String[] args) {
    System.out.println("Enter an Integer: ");
    int intValue = ScannerInput.getParsed(Integer::parseInt);
    System.out.println("Integer Value: " + intValue + "\n");
    
    System.out.println("Enter an Double: ");
    double doubleValue = ScannerInput.getParsed(Double::parseDouble);
    System.out.println("Double Value: " + doubleValue + "\n");
  }
}

I'm trying to avoid having the program crash due to user input while keeping it general purpose. Are there some edge cases I should adjust this code to look out for?

\$\endgroup\$
4
\$\begingroup\$

While it can sometimes be useful to expose only static members and not allow instances of your class to be constructed, I don't see what you're gaining by doing that here. But by not allowing multiple different ScannerInputs, you do lose out on the ability to have them operate on streams other than System.in. If you'd made a constructor public and the methods non-static, you could allow code like the following, which seems like it could be useful:

ScannerInput fileScanner = new ScannerInput(new FileInputStream(someFile));
int someInt = fileScanner.getParsed(Integer::parseInt);

I find it a bit strange how the code treats NumberFormatException differently from other exception types. You've made a class that makes it convenient to parse arbitrary values from the command line, but it acts completely differently when called as ScannerInput.getParsed(Double::parseDouble) than when called as, say, ScannerInput.getParsed(Paths::get). If you really want your code to keep reading lines until a parse succeeds, I think this is one of those rare situations where you're too specific about what you catch.

That said, I'm not sure error handling should be ScannerInput's responsibility in the first place. It might be more useful to just let the caller know that parsing failed and let them decode how to handle it. Maybe someone's writing a program targeted at a non-english-speaking audience and they don't want their error messages in english. Maybe they expect their program output to be piped into a different program and really don't want to write error messages to System.out. Letting the caller decide how to handle a failed parse could make your code more flexible.

That doesn't have to mean just letting exceptions propagate back to the caller, though I do think that's the better option. But if you prefer, you could perhaps keep the "retry until a parse succeeds" behaviour by having getParsed also take a Consumer<String> and toss any lines it fails to parse into that? It doesn't seem like an awful idea to be able to go

ScannerInput.getParsed(Integer::parseInt, (line) -> {
    System.err.printf("Please enter an integer, not \"%s\"\n", line);
});

With getParsed being implemented a bit like

public static <T> T getParsed(
        Function<String, T> function,
        Consumer<String> onFailure
    ) throws NoSuchElementException {

    T result = null;
    while(result == null) {
        String line = scanner.nextLine();
        try {
            result = function.apply(line);
        } catch (Exception ex) {
            onFailure.accept(line);
        }
    }
    return result;
}

This way, when a line fails to parse you might still be able to do something useful with it. Maybe you don't care very often, but it's nice to have options, even just something like putting it in a log somewhere. Though I do find that more awkward than something like

try {
    Path filePath = ScannerInput.getParsed(Paths.get);
} catch (InvalidPathException ex) {
    String line = ScannerInput.lastLine();
    System.err.printf("Invalid path: %s\n", line);
}

But since you can basically already do that by using Scanner::nextLine directly anyway, it probably would be nice to have access to a retrying variant.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your insights! Yeah, I initially set it up with only System.in since the idea came after reading one of the repeating questions about input issues with it over on StackOverflow so I was only thinking about that specific usage. I was also considering expanding the getParsed method to take a second parameter for specific error messages, but I had not considered doing so via a Consumer. That's brilliant, thank you for that! \$\endgroup\$
    – Tim Hunter
    Jun 9 at 13:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.