RuntimeExceptions

[d3xter]

I was wondering why you catch all exceptions and throw a RuntimeException instead.

[mwanji]

I normally do that for checked exceptions. I could check if the exception is already an unchecked exception and propagate it instead.

Do you have ant examples of where it's problematic?

[d3xter]

Its not really problematic, but we Java-users are not really used to Unchecked Exceptions ;)

One thing that could be considered is to have a single TomlException. No matter what exception occured inside Toml, we get a TomlException with getCause() set to whatever exception happened, e.g. FileNotFound, IOException, IllegalStateException, ...

So if toml4j starts throwing other exceptions later, a "catch (TomlException e)" is sufficient for the users of toml4j and they dont have to change the code.

[mwanji]

Personally, I hate checked exceptions for obvious stuff. If you're parsing a file that doesn't exist, you'll get a file not found exception at runtime. In practice, this is fine:

• applications know where their config files will be, so such an error is most likely a one-time mistake
• frameworks that receive file locations from users have to validate them anyway, so nothing is gained by toml4j declaring a checked exception

Parsing invalid TOML throws an IllegalStateException and I don't think it's worth making it a checked exception, as it's generally not something you can or want to recover from.

If I were to create a TomlException, it would be an unchecked exception, so it wouldn't help you ;)

[mwanji]

I will review the exception handling code to see if there's anything that can be improved, though, especially propagation.

[d3xter]

I expected TomlException to be a RuntimeException based on your prefering :D

What I mean is, the developer of an application usually just wants to know whether reading the configuration-file was successful. If not, he can display the error message and exit gracefully.

If he wants to know what exactly happened, he can still get the root Exception with "getCause()", but the other developers doesnt really have to care about what could possible go wrong. Catching IllegalStateException + RuntimeException opposed to just catching TomlException.

See: http://tutorials.jenkov.com/java-exception-handling/exception-wrapping.html

[d3xter]

One problem could arise from using IllegalState Exception:

1. IllegalStateException can be thrown outside of toml4j too
2. The developer didnt read the javadoc of toml4j properly
3. He catches IllegalStateException somewhere up the stack where both exceptions get caught

This could lead to strange bugs, because he didnt expect a toml exception to be catched there. Especially when he doesnt log the error message and is unable to debug when its on server.

And btw, IllegalStateException is wrong in this case ;) Javadoc:

Signals that a method has been invoked at an illegal or inappropriate time. In other words, the Java environment or Java application is not in an appropriate state for the requested operation.

IllegalFormatException would be more appropriate:

Unchecked exception thrown when a format string contains an illegal syntax or a format specifier that is incompatible with the given arguments. Only explicit subtypes of this exception which correspond to specific errors should be instantiated.

[mwanji]

Fair enough.

How about: throw an unchecked TomlException for parsing errors, but leave the I/O exceptions as part of the method signature.

eg. parse(File), parse(Reader) and parse(InputStream) would declare an IOException, but parse(String) would not.

[d3xter]

Sounds good.

I've thought it through again and i think it would be sufficient to use either IllegalFormatException or a TomlException, if it is only used for parsing errors.

IllegalStateException was misleading me :)

[Zero3]

Throwing in my opinion here, for what it is worth:

I personally try to avoid checked exceptions as well, and wrap any I encounter in a RuntimeException immediately. I prefer not having to maintain a complicated nesting of "throws" signatures throughout my application. If i intend to handle an exception, I will catch it explicitly. If not, I want the uncaught exception to bubble up through my application and eventually crash it with a stack trace.

So my preference is toml4j not throwing any checked exceptions at all, but of course documenting any RuntimeExceptions that might be thrown by its methods.