Proper way of handling exceptions while reading user input from file

Question

I am parsing a text file (UTF-8) and checking for several conditions that need to be satisfied for the code to be happy. If not, then there is no point to continue and the program should die.

At the moment I am catching exceptions and if caught, throw RuntimeException to signal the user that something went wrong.

What would be a proper way of doing this or is it OK the way it is?

/**
 * Reads the maze from plain text and converts it to char[][]
 * Marks the start (B) and end (F) coordinates if found, if not throws exception
 * Expects each line to have the same length as the the first line, throws exception if not
 *
 * @return maze as char[][]
 */
private char[][] readMazeFromFile() {
    List<String> stringList;
    try {
        stringList = Files.readAllLines(Paths.get("h40.in"), Charset.forName("UTF-8"));
    } catch (MalformedInputException e){
        throw new RuntimeException(String.format("Did not understand file, 
        check the charset, expecting UTF-8\n%s", e.getMessage()));
    } catch (IOException e) {
        throw new RuntimeException(String.format("Did not understand file, 
        sorry\n%s", e.getMessage()));
    } catch (Exception e){
        throw new RuntimeException(String.format
        ("Generic error while reading the input file, sorry\n%s", e));
    }

    char[][] charMaze = new char[stringList.size()][];
    for (int i = 0; i < stringList.size(); i++) {
        if (stringList.get(i).contains("B")) {
            startX = i;
            startY = stringList.get(i).indexOf("B");
        }
        if (stringList.get(i).contains("F")) {
            endX = i;
            endY = stringList.get(i).indexOf("F");
        }
        try {
            charMaze[i] = stringList.get(i).toCharArray();
        } catch (ArrayIndexOutOfBoundsException e) {
            throw new RuntimeException(String.format("Did not understand the maze,
            expecting square.\n%s", e.getMessage()));
        }

    }
    if (startX == -1 || startY == -1)
        throw new RuntimeException("Could not find starting point (B), aborting.");
    if (endX == -1 || endY == -1)
        throw new RuntimeException("Could not find ending point (F), aborting.");
    return charMaze;
}

Answer 1

I agree with @skiwi on the File input value. hardcoding the file name in this method is not helpful. I disagree with some of the Exception suggestions though...

IOExceptions are a special breed. If you cannot read the file, then you have to report it as an IOException. It irritates me that FileNotFoundException is an IOException though... so, I handle that one specially..... Otherwise, I tend to let IOExceptions 'escape' method calls. and the calling code should handle them... it's a bigger problem than the function.

Additionally, there is no reason to catch the broader Exception, and wrap it as a RuntimeException. There are no other thrown exceptions from the readLines method that are not RuntimeExceptions anyway....

Also, MalformedInputException is an IOException anyway... may as well handle them together.

Finally, your check to make sure the maze is square, does not. It should be checking the line-length to make sure it matches the expected size.... but it does not.

(Edit: Use StandardCharsets.UTF_8)

Here's how I would write your method:

/**
 * Reads the maze from plain text and converts it to char[][]
 * Marks the start (B) and end (F) coordinates if found, if not throws exception
 * Expects each line to have the same length as the the first line, throws exception if not
 *
 * @return maze as char[][]
 */
private char[][] readMazeFromFile(Path mazeFile) throws IOException {
    if (!Files.isRegularFile(mazeFile) || !Files.isReadable(mazeFile)) {
        throw new IllegalArgumentException("Cannot locate readable file " + mazeFile);
    }
    List<String> stringList = Files.readAllLines(mazeFile, StandardCharsets.UTF_8));

    char[][] charMaze = new char[stringList.size()][];
    for (String line : stringList) {
        if (line.length()  != charMaze.length) {
            throw new IllegalArgumentException("Expect the maze to be square, but line " + i + " is not " + charMaze.length + " characters long";
        }
        if (line.contains("B")) {
            startX = i;
            startY = line.indexOf("B");
        }
        if (line.contains("F")) {
            endX = i;
            endY = line.indexOf("F");
        }
        charMaze[i] = line.toCharArray();
    }
    if (startX == -1 || startY == -1)
        throw new IllegalArgumentException("Could not find starting point (B), aborting.");
    if (endX == -1 || endY == -1)
        throw new IllegalArgumentException("Could not find ending point (F), aborting.");
    return charMaze;
}

Answer 2

I don't think you are using a correct approach here unfortunately.
Due to time constraints I'll focus only on your main question, while the rest of the code may be worthwhile to review aswell.

You have a method called readMazeFromFile, it is expected to either return a char[][] or to signal that it failed to do so. It is not expected that it will abruptly kill the program.

I'll say upfront that I am not much of a fan for throwing checked exceptions, but I think it is appropiate in this case as you are handling user input.

It is appropiate to wrap most of the exceptions into a general (checked) wrapper exception that propagates the message, I am talking about MalformedInputException and IOException here. It is however not ok to catch Exception, as this will catch all exceptions, also runtime exceptions, hence hiding programmer errors. While this is not a big issue in your current setup due to several (lucky) factors, you should not be doing it ever.

One last piece of quick advice is that I would expect the readMazeFromFile method to have a File input parameter, instead of hardcoding it. Even better, you should go for the Path class which supersedes the File class.