Closed mikera closed 12 years ago
On a similar vein, I don't think Printer should extend Closeable either.....
The question is a more general one: do we want to expose checked exceptions, or not? I'm not a great fan of them, actually. Initially Parser did not throw checked exceptions. This changed, when I eliminated my home-grown Input abstraction and just had Scanner work directly on instances of Reader (which throws checked exceptions).
As currently designd, Parser and Printer are like the rest of the IO classes: they throw checked IOExceptions and they are Closeable. This is ugly, but it is idiomatic (Java).
I'll give the alternative of wrapping IOExceptions in a (subclass of) EdnException a try on a private branch and see if I like it better.
I'm so-so about checked exceptions themselves: they are annoying sometimes but in other cases can be useful when you want to really ensure that users handle important error cases.
I think the underlying issue here is that Parser and Printer shouldn't be depending on IO: this is "complecting" the concept of a parser / printer with an implementation detail (i.e. the use of readers / writers than may perform IO)
My view is that the API design would be better if we could abstract the interface away from the concrete implementation details. You could still have a Parser backed by a Reader as the default concrete implementation (perhaps this would any IOException in an EdnException), but the door would be open to other implementations.
Yes your previous comment hits on exactly what's wrong with checked exceptions: they leak implementation details into the public interface. I'll see what can be done by returning to my original idea (of exposing only unchecked exceptions at the public APIs).
One advantage of going the idiomatic route (IOException, Closeable) does occur to me: Parser and Printer should play nicely with Java-7's try-with-resources.
Sure we want to work nicely with try-with-resources, but that doesn't mean that Parser and Printer need to expose that detail: it should be the caller's job to use try-with-resources on whatever Reader / Writer is passed to the factory function to initialise the Parser / Printer.
I've pushed the branch topic/issue-11 which removes checked exceptions from the public APIs of Parser and Printer.
see also follow up issues: https://github.com/bpsm/edn-java/issues/14 https://github.com/bpsm/edn-java/issues/15
The printer interface currently declares checked exceptions (IOException).
I believe these should be removed for two main reasons: