Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-03 Thread Dimitrios Efthymiou
yes. You are right. Closed On Mon, 3 Jul 2023 at 10:12, Gilles Sadowski wrote: > Le lun. 3 juil. 2023 à 09:41, Alex Herbert a > écrit : > > > > On Mon, 3 Jul 2023 at 08:29, sebb wrote: > > > > > > Is null checked or unchecked? > > > > > > I think neither, so isUnchecked also needs to check

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-03 Thread Gilles Sadowski
Le lun. 3 juil. 2023 à 09:41, Alex Herbert a écrit : > > On Mon, 3 Jul 2023 at 08:29, sebb wrote: > > > > Is null checked or unchecked? > > > > I think neither, so isUnchecked also needs to check for null. > > > > I wonder whether it might be better to throw NPE in both cases for null. > > > >

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-03 Thread Alex Herbert
On Mon, 3 Jul 2023 at 08:29, sebb wrote: > > Is null checked or unchecked? > > I think neither, so isUnchecked also needs to check for null. > > I wonder whether it might be better to throw NPE in both cases for null. > > It may be confusing for users if not checked != unchecked. > e.g. it is

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-03 Thread sebb
Is null checked or unchecked? I think neither, so isUnchecked also needs to check for null. I wonder whether it might be better to throw NPE in both cases for null. It may be confusing for users if not checked != unchecked. e.g. it is tempting to code: if (isChecked(t)) { } else { // must be

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Elliotte Rusty Harold
On Mon, Jul 3, 2023 at 12:20 AM Gary Gregory wrote: > > Hi Elliotte: > > Might you be looking at some old code in the PR? > Just looking at what was posted in the email thread. It's a weird corner case not everyone thinks of. > The current code is: > > /** > * Checks if a throwable

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Dimitrios Efthymiou
I just noticed that the line return !isChecked(throwable) Means that if throwable is null then it will be considered unchecked. I will fix that tomorrow by doing Return throwable ! null && throwable instanceof Exception; On Mon, 3 Jul 2023, 01:20 Gary Gregory, wrote: > Hi Elliotte: > > Might

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Dimitrios Efthymiou
Hi Elliotte. I never thought of that, but I don't think it is Apache's problem if people exit the java convention On Mon, 3 Jul 2023, 01:02 Elliotte Rusty Harold, wrote: > On Sun, Jul 2, 2023 at 8:53 PM Alex Herbert > wrote: > > > public static boolean isUnchecked(final Throwable

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Gary Gregory
Hi Elliotte: Might you be looking at some old code in the PR? The current code is: /** * Checks if a throwable represents a checked exception * * @param throwable *The throwable to check. * @return True if the given Throwable is a checked exception.

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Elliotte Rusty Harold
On Sun, Jul 2, 2023 at 8:53 PM Alex Herbert wrote: > public static boolean isUnchecked(final Throwable throwable) { > return (throwable instanceof Error) || (throwable instanceof > RuntimeException); > } Not quite. It's also possible for someone to define a subclass of Throwable

Re: [commons-lang] branch master updated: [LANG-1647] Add and ExceptionUtils.isChecked() and isUnchecked() #1069

2023-07-02 Thread Alex Herbert
This change is not null-safe for isUnchecked. This case is missing from the tests (and currently fails): @Test public void testIsUnchecked_null() { assertFalse(ExceptionUtils.isUnchecked(null)); } The case fails because it simply negates isChecked. Both methods should return