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?
1 Answer 1
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 ScannerInput
s, 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.
-
\$\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 aConsumer
. That's brilliant, thank you for that! \$\endgroup\$Tim Hunter– Tim Hunter2021年06月09日 13:23:29 +00:00Commented Jun 9, 2021 at 13:23