I've created GEOMETRY-138 [1] to track this and discuss the technical details. -Matt
[1] https://issues.apache.org/jira/projects/GEOMETRY/issues/GEOMETRY-138 On Sat, Jul 24, 2021 at 12:41 PM Gilles Sadowski <gillese...@gmail.com> wrote: > > Le sam. 24 juil. 2021 à 18:25, Matt Juntunen > <matt.a.juntu...@gmail.com> a écrit : > > > > I don't have a preference for checked or unchecked exceptions. I just > > want it to be consistent, easy to use, and maintainable. > > Checked exceptions are a maintenance nightmare. > [Some time in the future, you change the implementation so that > it actually does not ever throw the (checked) exception; still you > are stuck with it because it's part of the signature.] > > > With that > > said, what specific changes are you picturing should be made to the > > code here? Are you suggesting a geometry-specific exception type? > > Should we wrap IOExceptions as runtime exceptions? > > Yes. > Fine to correct JDK mistakes, not to copy them. ;-) > > Here is a (7 years old) post that hopefully will, once and for all, settle > the recurring debate: > > https://literatejava.com/exceptions/checked-exceptions-javas-biggest-mistake/ > > If "TL;DR;" 2 quotes: > ---CUT--- > [...] if your projects are still using or advocating checked exceptions, > your skills are 5-10 years out [of] date. Java has moved on. > ---CUT--- > ---CUT--- > For checked exceptions to be useful, however, depends on selective > & usefully handleable (recoverable) exceptions being the only ones > checked. You could propose that FileNotFound or failures connecting > could be in that basket — but I/O failures reading or writing once the > file is open, are almost completely not. > ---CUT--- > > Thanks, > Gilles > > > > > -Matt > > > > On Sat, Jul 24, 2021 at 12:11 PM Gilles Sadowski <gillese...@gmail.com> > > wrote: > > > > > > Hi. > > > > > > Le sam. 24 juil. 2021 à 17:01, Matt Juntunen > > > <matt.a.juntu...@gmail.com> a écrit : > > > > > > > > Hello, > > > > > > > > > AFAICT, a precondition (max string length) is violated by the input: > > > > > the > > > > error is the caller's fault (either passing a too long string, or not > > > > having > > > > set an appropriate upper bound). > > > > > > > > This is not correct. This exception is thrown when a string token from > > > > the input stream exceeds the maximum string token length. (The maximum > > > > length is used to prevent invalid/malicious input from allocating an > > > > enormous string in the JVM.) So, the error is with the input, not the > > > > caller. > > > > > > There is a misunderstanding; in my statements, "caller" and "input" are > > > used interchangeably, and synonyms for "precondition violation", IOW: > > > "This method assumes <this> and <that>, and if you call it by passing > > > input that is not compliant, it will throw an exception". > > > In the above sentence, a _runtime_ exception is thrown if the error > > > is not recoverable. > > > > > > Note that "recoverable" would imply that the caller (*within* [Geometry]) > > > catches the (checked) exception and is able to complete the requested > > > task. [I don't know how that's possible in this instance (where input has > > > been identified as wrong).] > > > > > > > The general principle with the geometry IO code is that IOExceptions > > > > (which the methods in question must already declare since they > > > > ultimately read from InputStreams) indicate an error extracting data > > > > from the input. Possible cases include: > > > > - network/file system errors > > > > - parsing/syntax errors > > > > - data conversion errors (e.g. string to double) > > > > > > None of those are recoverable from the POV the [Geometry] code: > > > Parsing having failed for any of those reason will raise an exception > > > and it's up to the caller to sort it > > > There is no added value for this exception to be checked by the > > > compiler. > > > Going against currently recommended practice (cf. e.g. "Effective > > > Java" by J. Bloch) would only be valid for BC reasons (cf. recent > > > discussion about [Compress]). > > > > > > > Runtime exceptions may be thrown for other issues occurring after the > > > > raw data has been retrieved from the file/stream, primarily if the > > > > extracted data is not mathematically valid. > > > > > > Example (b) does the opposite: It turns a runtime exception into an > > > "IOException". > > > > > > I note that parsing occurs _after_ reading; such that any "IOException" > > > that is raised from an underlying stream should (IMO) be caught ASAP > > > and wrapped in a runtime exception (to be rethrown, since the condition > > > is not recoverable). > > > The root cause (e.g. whether it is a parse issue or a file system issue) > > > will be visible in the stack trace. No need to spread "throws" clauses > > > for that. > > > > > > > I feel that this separation of exception behavior between IO/format > > > > errors and high-level mathematical validation is useful. > > > > > > I agree, but "different behaviours" can be implemented by different > > > types. This is orthogonal to "checked" vs "unchecked". > > > > > > > As an end > > > > user of an application that uses this library, I want to know if the > > > > file I provided was just plain invalid versus if it just contained a > > > > wonky geometry. > > > > > > I'm not sure I understand the issue (it seems to me that both are > > > irrecoverable). Please provide an example that mandates checked > > > exceptions. > > > > > > Regards, > > > Gilles > > > > > > > > > > > Regards, > > > > Matt > > > > > > > > On Sat, Jul 24, 2021 at 8:28 AM Gilles Sadowski <gillese...@gmail.com> > > > > wrote: > > > > > > > > > > Hello. > > > > > > > > > > Somehow I missed that [Geometry] contains usage of checked exceptions. > > > > > As mentioned in other threads, I have a hard time conceiving that the > > > > > kind > > > > > of codes we are dealing with here ever needs the concept of checked > > > > > exception (in its intended usage per the _current_ Java experts, and > > > > > not > > > > > based on outdated practice from the time when Java advocates were > > > > > hoping > > > > > that checked exceptions would be the cure to all evil...). > > > > > > > > > > Examples: > > > > > > > > > > (a) > > > > > ---CUT--- > > > > > /** Get the string collected by this instance. > > > > > * @return the string collected by this instance > > > > > * @throws IOException if the string exceeds the maximum > > > > > configured length > > > > > */ > > > > > public String getString() throws IOException { > > > > > if (hasExceededMaxStringLength()) { > > > > > throw parseError(line, col, STRING_LENGTH_ERR_MSG + > > > > > maxStringLength); > > > > > } > > > > > return sb.toString(); > > > > > } > > > > > ---CUT--- > > > > > > > > > > AFAICT, a precondition (max string length) is violated by the input: > > > > > the > > > > > error is the caller's fault (either passing a too long string, or not > > > > > having > > > > > set an appropriate upper bound). In any case, it is not recoverable, > > > > > so > > > > > a checked exception is ruled out. > > > > > > > > > > (b) > > > > > ---CUT--- > > > > > /** Get the current token parsed as an integer. > > > > > * @return the current token parsed as an integer > > > > > * @throws IllegalStateException if no token has been read > > > > > * @throws IOException if the current token cannot be parsed as > > > > > an integer > > > > > */ > > > > > public int getCurrentTokenAsInt() throws IOException { > > > > > ensureHasSetToken(); > > > > > Throwable cause = null; > > > > > if (currentToken != null) { > > > > > try { > > > > > return Integer.parseInt(currentToken); > > > > > } catch (NumberFormatException exc) { > > > > > cause = exc; > > > > > } > > > > > } > > > > > throw unexpectedToken("integer", cause); > > > > > } > > > > > ---CUT--- > > > > > > > > > > "Integer.parseInt" (JDK) complies with the rationale: a runtime > > > > > exception > > > > > is rightfully thrown on precondition failure (input cannot be parsed > > > > > as an > > > > > "integer"), and turning it into a checked exception just adds > > > > > complexity > > > > > for zero gain (such errors are all the same non-recoverable). > > > > > > > > > > For the parser to be robust (in the sense of not crashing the code > > > > > without a > > > > > message that correctly identifies the cause of the failure), there is > > > > > no need > > > > > for checked exceptions. > > > > > > > > > > IMO, for any new code, the introduction of a checked exception should > > > > > be > > > > > accompanied by an example demonstrating what the library does in order > > > > > to _recover_. [If it doesn't to anything (other than throw, rethrow, > > > > > or log an > > > > > error message) then an unchecked exception should be used instead.] > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
