I don't have a preference for checked or unchecked exceptions. I just want it to be consistent, easy to use, and maintainable. 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?
-Matt On Sat, Jul 24, 2021 at 12:11 PM Gilles Sadowski <[email protected]> wrote: > > Hi. > > Le sam. 24 juil. 2021 à 17:01, Matt Juntunen > <[email protected]> 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 <[email protected]> > > 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.] > > > > > > > > > Regads, > > > Gilles > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [email protected] > > > For additional commands, e-mail: [email protected] > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
