commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gillese...@gmail.com>
Subject Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions to [lang].
Date Thu, 05 Sep 2019 15:43:44 GMT
Le jeu. 5 sept. 2019 à 17:37, Gary Gregory <garydgregory@gmail.com> a écrit :
>
> On Thu, Sep 5, 2019 at 4:33 AM Gilles Sadowski <gilleseran@gmail.com> wrote:
>
> > Le jeu. 5 sept. 2019 à 02:50, Gary Gregory <garydgregory@gmail.com> a
> > écrit :
> > >
> > > On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xenoamess@gmail.com> wrote:
> > >
> > > > Well, if you do think repeating the similar codes for 100+ times is
> > > > better than reflection and be more elegant or easier to read or
> > > > something, then you can try maintain them.
> > > > There is 100+ Throwable classes who have string constructor in JDK IMO.
> > > > And don't forget some of them might only be in some certain versions
> > > > of JDK, and be careful about making them compilable on other version
> > > > of JDKs.
> > > > Also you might try to use some preprocess way to generate such
> > > > classes, some ways like lombok do (although that might be
> > > > controversial).
> > > >
> > >
> > > Allow me to repeat, perhaps rephrase, and elucidate any miscommunication
> > > based on what I said in the PR here:
> > > https://github.com/apache/commons-lang/pull/450#issuecomment-527892606
> > >
> > > I am _not_ suggesting to apply this pattern to 100+ exception classes,
> > > ever. This is really an ever stricter application of the YAGNI and 80/20
> > > rules. Both this idea and Commons Lang do not intend to provide this kind
> > > of comprehensive coverage. This PR is for one exception and eats its own
> > > dog food by refactoring 52 existing call sites within Commons Lang to use
> > > the new methods. I then suggested the same could applied for
> > > NullPointerException (7 possible call sites in Lang.) and
> > > IllegalStateException (29 possible call sites in Lang.) but I am not
> > doing
> > > as far as implementing this, this could be done sooner or later. I really
> > > think that by covering these three cases, we would fall into the 80/20
> > > rule. In addition, there is reason to cover other exceptions like JDBC's
> > > SQLException family of classes, but that's a job for Commons DbUtils or
> > > Commons DBCP, not Commons Lang.
> > >
> >
> > There is still the issue of the API itself (cf. some posts upwards), unless
> > your last comment implies that this is going to go in an "internal"
> > package.
> >
>
> The intent is not for an internal package, but for a public API, which you
> can see from the PR since it is in the existing 'exception' package, as
> opposed to a new 'internal' package.

I thought so, but then my earlier comment stands (about the API).

> You will also see that the new class is used from all over Commons Lang,
> therefore making it impossible to use a package private class.
> While there may be room for a lambda styled (not reflection)
> implementation, the call sites would be uglier and harder to maintain. I
> would be OK with adding ExceptionUtils with lambdas with the
> ExceptionClass::new style but this is separate IMO. The call site should
> pick which API it wants.

The point was about finding the right API to add here.

Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message