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 <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.]
> > >
> > >
> > > Regads,
> > > Gilles
> > >
> > > ---------------------------------------------------------------------
> > > 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
> >
>
> ---------------------------------------------------------------------
> 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

Reply via email to