HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaModerate

Proper way of handling exceptions while reading user input from file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingexceptionsreadingwhilefileuserwayinputproperfrom

Problem

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 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)

Solution

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 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;
}

Code Snippets

/**
 * 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;
}

Context

StackExchange Code Review Q#51386, answer score: 10

Revisions (0)

No revisions yet.