Correct usage of IllegalArgumentException

Question

Original question is here

I am reading in a UTF-8 file and parsing the contents of that file. If there is error in the file there is no point to continue and execution should stop. I have been suggested to throw IllegalArgumentException if there are problems with the contents, but the API doc says:

Thrown to indicate that a method has been passed an illegal or inappropriate argument.

In my code, the argument would be the file (or actually the path) that I pass, is it correct to throw IllegalArgumentException in case something goes wrong while parsing? If not, what type of exception should I throw?

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 (int i = 0; i < stringList.size(); i++) {
        String line = stringList.get(i);
        if (line.length() != charMaze.length)
            throw new IllegalArgumentException(String.format("Expect the maze to be square, but line %d is not %d characters long", line.length(), charMaze.length));
        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;
}

No in those cases you should not throw an IllegalArgumentException. Maybe a custom Exception like MazeParseException (like the SAXException when using DocumentBuilder.parse()). Use IllegalArgumentExceptions when you don't want to accept nulls but a null was given or if you require a folder but a file was given, etc.. — Icewind, May 22 '14 at 14:31
I'd say this question is verging on primarily opinion based. I personally wouldn't bother creating a new exception class in this case, I'd just throw a `RuntimeException` with an appropriate error message. — JonK, May 22 '14 at 14:34
IllegalArgumentException is perfectly fine here. The Path object doesn't meet the requirements of the method. — Oliver Watkins, May 22 '14 at 14:35

score 5 · Accepted Answer · answered May 22 '14 at 14:52

I think the first usage is correct:

if (!Files.isRegularFile(mazeFile) || !Files.isReadable(mazeFile)) {
    throw new IllegalArgumentException("Cannot locate readable file "+mazeFile);
}

Since (as the documentation states) an invalid file was provided as the argument, this should throw an IllegalArgumentException. Once you know you have an actual file that meets those requirements, I personally don't think that is a good exception to throw. This will cause other developers to question the type of argument that was given as opposed to the contents of the file. I guess your options are:

Keep it as is, just with very specific error messages explaining why this was an invalid argument.
Use some other, potentially more applicable java exception such as java.text.ParseException, since it is the file parsing that is causing the error.
Create a custom exception class that more sufficiently describes the issue with the file, e.g. a MazeParseException (per the comments) or a FileFormatException.

I would expect the second or third option to be more beneficial if you anticipate several other developers executing your function.

score 1 · Answer 2 · answered May 22 '14 at 14:54

Exceptions are mainly nothing but names, My best advice here would be to make your own. Primary reason being that IllegalArgumentExceptions are unchecked exceptions because they extend java.lang.RuntimeException. They would just cause problems if you used them in a context like this. (Source)

Change the method signature to

private char[][] readMazeFromFile(Path mazeFile) throws IOException, MazeParseException {...}

And all throw new IllegalArgumentException with throw new MazeParseException (except for the first usage as per @Joel's answer)

The MazeParseException.java file:

package yourPackage.here;

import java.lang.Exception;

public class MazeParseException {
    public MazeParseException() {
        super();
    }

    public MazeParseException(String reason) {
        super(reason);
    }
}

The benefits here of using your own exception is that you can tag extra data along with the exception relating to your situation, for example you could add:

private int errorLineNum = null;
public MazeParseException(String reason, int lineNum) {
    this(reason);
    this.errorLineNum = lineNum;
}

public int getMalformedLine() {
    return this.errorLineNum;
}

// And then update the toString() method to incorperate the errorLineNum
@Override
public String toString() {
    StringBuilder sb = new StringBuilder(super.toString());
    if(errorLineNum != null) {
        sb.append("@ line #");
        sb.append(this.errorLineNum);
    }
    return sb.toString();

}

Eliott Roynette · Answer 3 · 2014-05-22T15:02:34.890

As JSON or XML library send their own execption if the file doesn't match what they are looking for (a JSON file or a XML one), I think you should do the same if doesn't match what you are looking for( an UTF-8 file ).

IllegalArgumentException should be use for internal problem in your code and should not be throw when your code have been debug. Also you should not catch an IllegalArgumentException and you will probably want to catch it in your program.

Correct usage of IllegalArgumentException

3 Answers3