You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2019/09/03 15:18:34 UTC
[lang] org.apache.commons.text.lookup.IllegalArgumentExceptions to [lang].
Hi All:
I propose we take
https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
and make it a public class in [lang]. FWIW, I use a class like this in many
projects at work.
Gary
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Rob Spoor <ap...@icemanx.nl>.
That Builder can throw an unexpected exception if you forget to call
withFormat, because the format and args remain null.
Regarding the reflection code, the null checks are unnecessary.
getDeclaredConstructor cannot return null, it throws an exception
instead if the constructor cannot be found.
On 04/09/2019 16:22, Gilles Sadowski wrote:
> Le mer. 4 sept. 2019 à 14:34, Xeno Amess <xe...@gmail.com> a écrit :
>>
>> No I still don't think it a good idea to have such a util class.
>> 1. most Throwable classes, they all have a constructor with a String.
>> 2. IllegalArgumentException is not special in this way.
>
> For a library such as [Lang], that seems quite right.
>
>> 3. If we make a public Util class for IllegalArgumentException, then
>> we shall make similar public Util classes for all other Throwables in
>> JDK.
>
> Or a single factory class that can create all (untested code):
>
> ---CUT---
> public class ExceptionFactory {
> private ExceptionFactory() {}
>
> public static IllegalArgumentBuilder illegalArgument() {
> return new IllegalArgumentBuilder();
> }
>
> private abstract static class Builder<T extends Throwable> {
> private final String format;
> private final Object formatArgs;
>
> protected Builder() {
> this(null, null);
> }
> private Builder(String format, Object[] formatArgs) {
> this.format = format;
> this.args = formatArgs;
> }
> public Builder withFormat(String format, Object ... args) {
> return new Builder(format, args);
> }
> protected String formatMessage() {
> return String.format(format, formatArgs);
> }
> protected abstract T build();
> }
>
> public static class IllegalArgumentBuilder extends
> Builder<IllegalArgumentException> {
> @Override
> public IllegalArgumentException build() {
> return new IllegalArgumentException(formatMessage());
> }
> }
> }
> ---CUT---
>
> Usage would be:
> ---CUT---
> throw ExceptionFactory.illegalArgument().withFormat(format, args).build();
> ---CUT---
>
>> However if you really want to add something like that, I think
>> something like this would be a quite reasonable answer:
>>
>>
>> import org.junit.jupiter.api.Test;
>>
>> import java.lang.reflect.Constructor;
>> import java.lang.reflect.InvocationTargetException;
>> import java.util.Arrays;
>>
>> import static org.junit.jupiter.api.Assertions.assertEquals;
>>
>> public class ExceptionUtils {
>> public static <T> T generateException(final Class<T>
>> exceptionClass, final String format,
>> final Object... arguments) {
>> T result = null;
>> try {
>> Constructor<T> exceptionConstructor =
>> exceptionClass.getDeclaredConstructor(String.class);
>> if (exceptionConstructor != null) {
>> result =
>> exceptionConstructor.newInstance(String.format(format, arguments));
>> }
>> } catch (NoSuchMethodException | InstantiationException |
>> IllegalAccessException | InvocationTargetException e) {
>> throw generateException(IllegalArgumentException.class, e,
>> "Can't create %s with arguments %s.",
>> exceptionClass.getCanonicalName(),
>> Arrays.toString(arguments));
>> }
>> return result;
>> }
>>
>> public static <T> T generateException(final Class<T>
>> exceptionClass, Throwable cause, final String format,
>> final Object... arguments) {
>> T result = null;
>> try {
>> Constructor<T> exceptionConstructor =
>> exceptionClass.getDeclaredConstructor(String.class, Throwable.class);
>> if (exceptionConstructor != null) {
>> result =
>> exceptionConstructor.newInstance(String.format(format, arguments),
>> cause);
>> }
>> } catch (NoSuchMethodException | InstantiationException |
>> IllegalAccessException | InvocationTargetException e) {
>> throw generateException(IllegalArgumentException.class, e,
>> "Can't create %s with arguments %s.",
>> exceptionClass.getCanonicalName(),
>> Arrays.toString(arguments));
>> }
>> return result;
>> }
>
> Seems fragile to risk generating an exception other than the expected
> one (IIUC).
>
> Regards,
> Gilles
>
>>
>> @Test
>> public void test() {
>> RuntimeException re =
>> generateException(RuntimeException.class, "%d,,%s", 1, "2020");
>> assertEquals(re.getMessage(), "1,,2020");
>> RuntimeException re2 =
>> generateException(RuntimeException.class, re, "%d,,%s", 2, "3030");
>> assertEquals(re2.getMessage(), "2,,3030");
>> assertEquals(re2.getCause(), re);
>> }
>> }
>>
>> Gilles Sadowski <gi...@gmail.com> 于2019年9月4日周三 下午7:17写道：
>>>
>>> Hi.
>>>
>>> Le mer. 4 sept. 2019 à 02:53, Gary Gregory <ga...@gmail.com> a écrit :
>>>>
>>>> Here is the PR with [lang]'s own call sites updated to use the new code.
>>>>
>>>> Gary
>>>>
>>>> On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com> wrote:
>>>>
>>>>> On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
>>>>>
>>>>>> Why limit this to IllegalArgument? Why not make it more generic? For
>>>>>> instance, in ExceptionUtils:
>>>>>>
>>>>>> public static <T extends Throwable> T format(final Function<? super
>>>>>> String, ? extends T> factory, final String format, final Object... args) {
>>>>>> return factory.apply(String.format(format, args));
>>>>>> }
>>>>>>
>>>>>> public static <T extends Throwable> T format(final BiFunction<? super
>>>>>> String, ? super Throwable, ? extends T> factory, final Throwable t,
>>>>>> final String format, final Object... args) {
>>>>>> return factory.apply(String.format(format, args), t);
>>>>>> }
>>>>>>
>>>>>> These can then be called using
>>>>>> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
>>>>>> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
>>>>>> "message: %s", message).
>>>
>>> API looks a little bit strange (throw a "format"?)
>>>
>>> Perhaps:
>>> ---CUT---
>>> throw IllegalArgumentExceptionFactory.withFormat(...);
>>> ---CUT---
>>>
>>> Is there additional customization foreseen (that may require
>>> a "builder")?
>>>
>>> Also, wouldn't it be useful to not perform the formatting
>>> unconditionally but rather when "getMessage() is called?
>>>
>>> Gilles
>>>
>>>>>>
>>>>>> It's a bit verbose though, but it gives a lot more flexibility.
>>>>>>
>>>>>
>>>>> Yes, we could add these to ExceptionUtils separately IMO; but, the
>>>>> verbosity is an issue for me. This is straightforward:
>>>>>
>>>>> throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
>>>>> source, method, pathSpec, operation.getOperationId(), e.toString());
>>>>>
>>>>> The following not as much:
>>>>>
>>>>> throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
>>>>> operation %s: %s", source, method, pathSpec, operation.getOperationId(),
>>>>> e.toString());
>>>>>
>>>>> So it could be that IllegalArgumentExceptions.format() is implemented in
>>>>> terms of ExceptionUtils.format but there does not seem to be much to gain
>>>>> from that.
>>>>>
>>>>> Gary
>>>>>
>>>>>
>>>>>>
>>>>>> On 03/09/2019 19:04, Gary Gregory wrote:
>>>>>>> Please read the source.
>>>>>>>
>>>>>>> On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
>>>>>>>
>>>>>>>> Why don't you use java.lang.IllegalArgumentException instead?
>>>>>>>> And, why we need such a class, but not using
>>>>>>>> java.lang.IllegalArgumentException there?
>>>>>>>>
>>>>>>>> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
>>>>>>>>>
>>>>>>>>> Hi All:
>>>>>>>>>
>>>>>>>>> I propose we take
>>>>>>>>>
>>>>>>>>
>>>>>> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
>>>>>>>>> and make it a public class in [lang]. FWIW, I use a class like this in
>>>>>>>> many
>>>>>>>>> projects at work.
>>>>>>>>>
>>>>>>>>> Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mer. 4 sept. 2019 à 14:34, Xeno Amess <xe...@gmail.com> a écrit :
>
> No I still don't think it a good idea to have such a util class.
> 1. most Throwable classes, they all have a constructor with a String.
> 2. IllegalArgumentException is not special in this way.
For a library such as [Lang], that seems quite right.
> 3. If we make a public Util class for IllegalArgumentException, then
> we shall make similar public Util classes for all other Throwables in
> JDK.
Or a single factory class that can create all (untested code):
---CUT---
public class ExceptionFactory {
private ExceptionFactory() {}
public static IllegalArgumentBuilder illegalArgument() {
return new IllegalArgumentBuilder();
}
private abstract static class Builder<T extends Throwable> {
private final String format;
private final Object formatArgs;
protected Builder() {
this(null, null);
}
private Builder(String format, Object[] formatArgs) {
this.format = format;
this.args = formatArgs;
}
public Builder withFormat(String format, Object ... args) {
return new Builder(format, args);
}
protected String formatMessage() {
return String.format(format, formatArgs);
}
protected abstract T build();
}
public static class IllegalArgumentBuilder extends
Builder<IllegalArgumentException> {
@Override
public IllegalArgumentException build() {
return new IllegalArgumentException(formatMessage());
}
}
}
---CUT---
Usage would be:
---CUT---
throw ExceptionFactory.illegalArgument().withFormat(format, args).build();
---CUT---
> However if you really want to add something like that, I think
> something like this would be a quite reasonable answer:
>
>
> import org.junit.jupiter.api.Test;
>
> import java.lang.reflect.Constructor;
> import java.lang.reflect.InvocationTargetException;
> import java.util.Arrays;
>
> import static org.junit.jupiter.api.Assertions.assertEquals;
>
> public class ExceptionUtils {
> public static <T> T generateException(final Class<T>
> exceptionClass, final String format,
> final Object... arguments) {
> T result = null;
> try {
> Constructor<T> exceptionConstructor =
> exceptionClass.getDeclaredConstructor(String.class);
> if (exceptionConstructor != null) {
> result =
> exceptionConstructor.newInstance(String.format(format, arguments));
> }
> } catch (NoSuchMethodException | InstantiationException |
> IllegalAccessException | InvocationTargetException e) {
> throw generateException(IllegalArgumentException.class, e,
> "Can't create %s with arguments %s.",
> exceptionClass.getCanonicalName(),
> Arrays.toString(arguments));
> }
> return result;
> }
>
> public static <T> T generateException(final Class<T>
> exceptionClass, Throwable cause, final String format,
> final Object... arguments) {
> T result = null;
> try {
> Constructor<T> exceptionConstructor =
> exceptionClass.getDeclaredConstructor(String.class, Throwable.class);
> if (exceptionConstructor != null) {
> result =
> exceptionConstructor.newInstance(String.format(format, arguments),
> cause);
> }
> } catch (NoSuchMethodException | InstantiationException |
> IllegalAccessException | InvocationTargetException e) {
> throw generateException(IllegalArgumentException.class, e,
> "Can't create %s with arguments %s.",
> exceptionClass.getCanonicalName(),
> Arrays.toString(arguments));
> }
> return result;
> }
Seems fragile to risk generating an exception other than the expected
one (IIUC).
Regards,
Gilles
>
> @Test
> public void test() {
> RuntimeException re =
> generateException(RuntimeException.class, "%d,,%s", 1, "2020");
> assertEquals(re.getMessage(), "1,,2020");
> RuntimeException re2 =
> generateException(RuntimeException.class, re, "%d,,%s", 2, "3030");
> assertEquals(re2.getMessage(), "2,,3030");
> assertEquals(re2.getCause(), re);
> }
> }
>
> Gilles Sadowski <gi...@gmail.com> 于2019年9月4日周三 下午7:17写道：
> >
> > Hi.
> >
> > Le mer. 4 sept. 2019 à 02:53, Gary Gregory <ga...@gmail.com> a écrit :
> > >
> > > Here is the PR with [lang]'s own call sites updated to use the new code.
> > >
> > > Gary
> > >
> > > On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com> wrote:
> > >
> > > > On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
> > > >
> > > >> Why limit this to IllegalArgument? Why not make it more generic? For
> > > >> instance, in ExceptionUtils:
> > > >>
> > > >> public static <T extends Throwable> T format(final Function<? super
> > > >> String, ? extends T> factory, final String format, final Object... args) {
> > > >> return factory.apply(String.format(format, args));
> > > >> }
> > > >>
> > > >> public static <T extends Throwable> T format(final BiFunction<? super
> > > >> String, ? super Throwable, ? extends T> factory, final Throwable t,
> > > >> final String format, final Object... args) {
> > > >> return factory.apply(String.format(format, args), t);
> > > >> }
> > > >>
> > > >> These can then be called using
> > > >> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
> > > >> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
> > > >> "message: %s", message).
> >
> > API looks a little bit strange (throw a "format"?)
> >
> > Perhaps:
> > ---CUT---
> > throw IllegalArgumentExceptionFactory.withFormat(...);
> > ---CUT---
> >
> > Is there additional customization foreseen (that may require
> > a "builder")?
> >
> > Also, wouldn't it be useful to not perform the formatting
> > unconditionally but rather when "getMessage() is called?
> >
> > Gilles
> >
> > > >>
> > > >> It's a bit verbose though, but it gives a lot more flexibility.
> > > >>
> > > >
> > > > Yes, we could add these to ExceptionUtils separately IMO; but, the
> > > > verbosity is an issue for me. This is straightforward:
> > > >
> > > > throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
> > > > source, method, pathSpec, operation.getOperationId(), e.toString());
> > > >
> > > > The following not as much:
> > > >
> > > > throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
> > > > operation %s: %s", source, method, pathSpec, operation.getOperationId(),
> > > > e.toString());
> > > >
> > > > So it could be that IllegalArgumentExceptions.format() is implemented in
> > > > terms of ExceptionUtils.format but there does not seem to be much to gain
> > > > from that.
> > > >
> > > > Gary
> > > >
> > > >
> > > >>
> > > >> On 03/09/2019 19:04, Gary Gregory wrote:
> > > >> > Please read the source.
> > > >> >
> > > >> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
> > > >> >
> > > >> >> Why don't you use java.lang.IllegalArgumentException instead?
> > > >> >> And, why we need such a class, but not using
> > > >> >> java.lang.IllegalArgumentException there?
> > > >> >>
> > > >> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> > > >> >>>
> > > >> >>> Hi All:
> > > >> >>>
> > > >> >>> I propose we take
> > > >> >>>
> > > >> >>
> > > >> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> > > >> >>> and make it a public class in [lang]. FWIW, I use a class like this in
> > > >> >> many
> > > >> >>> projects at work.
> > > >> >>>
> > > >> >>> Gary
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by sebb <se...@gmail.com>.
I've just reviewed the PR, and I think it has a lot of issues (typos,
wrong format types etc.)
It also mixes different concerns.
As such, I am -1 to the PR as it stands.
I also don't think the new class adds any value.
AFAICT all it does is allow one to drop the text 'new ' and 'String.'.
(at the cost of an extra import)
On Thu, 5 Sep 2019 at 09:33, Gilles Sadowski <gi...@gmail.com> wrote:
>
> Le jeu. 5 sept. 2019 à 02:50, Gary Gregory <ga...@gmail.com> a écrit :
> >
> > On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xe...@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.
>
> Regards,
> Gilles
>
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gilles Sadowski <gi...@gmail.com>.
Le jeu. 5 sept. 2019 à 17:37, Gary Gregory <ga...@gmail.com> a écrit :
>
> On Thu, Sep 5, 2019 at 4:33 AM Gilles Sadowski <gi...@gmail.com> wrote:
>
> > Le jeu. 5 sept. 2019 à 02:50, Gary Gregory <ga...@gmail.com> a
> > écrit :
> > >
> > > On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xe...@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
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Sep 5, 2019 at 4:33 AM Gilles Sadowski <gi...@gmail.com> wrote:
> Le jeu. 5 sept. 2019 à 02:50, Gary Gregory <ga...@gmail.com> a
> écrit :
> >
> > On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xe...@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.
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.
Gary
>
> Regards,
> Gilles
>
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gilles Sadowski <gi...@gmail.com>.
Le jeu. 5 sept. 2019 à 02:50, Gary Gregory <ga...@gmail.com> a écrit :
>
> On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xe...@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.
Regards,
Gilles
> Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 4, 2019 at 4:51 PM Xeno Amess <xe...@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.
Gary
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Xeno Amess <xe...@gmail.com>.
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).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 4, 2019 at 8:34 AM Xeno Amess <xe...@gmail.com> wrote:
> No I still don't think it a good idea to have such a util class.
> 1. most Throwable classes, they all have a constructor with a String.
> 2. IllegalArgumentException is not special in this way.
> 3. If we make a public Util class for IllegalArgumentException, then
> we shall make similar public Util classes for all other Throwables in
> JDK.
> However if you really want to add something like that, I think
> something like this would be a quite reasonable answer:
>
>
> import org.junit.jupiter.api.Test;
>
> import java.lang.reflect.Constructor;
> import java.lang.reflect.InvocationTargetException;
> import java.util.Arrays;
>
> import static org.junit.jupiter.api.Assertions.assertEquals;
>
> public class ExceptionUtils {
> public static <T> T generateException(final Class<T>
> exceptionClass, final String format,
> final Object... arguments) {
> T result = null;
> try {
> Constructor<T> exceptionConstructor =
> exceptionClass.getDeclaredConstructor(String.class);
> if (exceptionConstructor != null) {
> result =
> exceptionConstructor.newInstance(String.format(format, arguments));
> }
> } catch (NoSuchMethodException | InstantiationException |
> IllegalAccessException | InvocationTargetException e) {
> throw generateException(IllegalArgumentException.class, e,
> "Can't create %s with arguments %s.",
> exceptionClass.getCanonicalName(),
> Arrays.toString(arguments));
> }
> return result;
> }
>
> public static <T> T generateException(final Class<T>
> exceptionClass, Throwable cause, final String format,
> final Object... arguments) {
> T result = null;
> try {
> Constructor<T> exceptionConstructor =
> exceptionClass.getDeclaredConstructor(String.class, Throwable.class);
> if (exceptionConstructor != null) {
> result =
> exceptionConstructor.newInstance(String.format(format, arguments),
> cause);
> }
> } catch (NoSuchMethodException | InstantiationException |
> IllegalAccessException | InvocationTargetException e) {
> throw generateException(IllegalArgumentException.class, e,
> "Can't create %s with arguments %s.",
> exceptionClass.getCanonicalName(),
> Arrays.toString(arguments));
> }
> return result;
> }
>
> @Test
> public void test() {
> RuntimeException re =
> generateException(RuntimeException.class, "%d,,%s", 1, "2020");
> assertEquals(re.getMessage(), "1,,2020");
> RuntimeException re2 =
> generateException(RuntimeException.class, re, "%d,,%s", 2, "3030");
> assertEquals(re2.getMessage(), "2,,3030");
> assertEquals(re2.getCause(), re);
> }
> }
>
Just reading the code, this feels error prone and very hard to read,
especially compared to the examples in the PR I created, all of this on top
of using reflection which should be a code smell here IMO.
Gary
> Gilles Sadowski <gi...@gmail.com> 于2019年9月4日周三 下午7:17写道：
> >
> > Hi.
> >
> > Le mer. 4 sept. 2019 à 02:53, Gary Gregory <ga...@gmail.com> a
> écrit :
> > >
> > > Here is the PR with [lang]'s own call sites updated to use the new
> code.
> > >
> > > Gary
> > >
> > > On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com>
> wrote:
> > >
> > > > On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
> > > >
> > > >> Why limit this to IllegalArgument? Why not make it more generic? For
> > > >> instance, in ExceptionUtils:
> > > >>
> > > >> public static <T extends Throwable> T format(final Function<? super
> > > >> String, ? extends T> factory, final String format, final Object...
> args) {
> > > >> return factory.apply(String.format(format, args));
> > > >> }
> > > >>
> > > >> public static <T extends Throwable> T format(final BiFunction<?
> super
> > > >> String, ? super Throwable, ? extends T> factory, final Throwable t,
> > > >> final String format, final Object... args) {
> > > >> return factory.apply(String.format(format, args), t);
> > > >> }
> > > >>
> > > >> These can then be called using
> > > >> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
> > > >> message) or ExceptionUtils.format(IllegalArgumentException::new,
> cause,
> > > >> "message: %s", message).
> >
> > API looks a little bit strange (throw a "format"?)
> >
> > Perhaps:
> > ---CUT---
> > throw IllegalArgumentExceptionFactory.withFormat(...);
> > ---CUT---
> >
> > Is there additional customization foreseen (that may require
> > a "builder")?
> >
> > Also, wouldn't it be useful to not perform the formatting
> > unconditionally but rather when "getMessage() is called?
> >
> > Gilles
> >
> > > >>
> > > >> It's a bit verbose though, but it gives a lot more flexibility.
> > > >>
> > > >
> > > > Yes, we could add these to ExceptionUtils separately IMO; but, the
> > > > verbosity is an issue for me. This is straightforward:
> > > >
> > > > throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s:
> %s",
> > > > source, method, pathSpec, operation.getOperationId(), e.toString());
> > > >
> > > > The following not as much:
> > > >
> > > > throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s:
> %s %s
> > > > operation %s: %s", source, method, pathSpec,
> operation.getOperationId(),
> > > > e.toString());
> > > >
> > > > So it could be that IllegalArgumentExceptions.format() is
> implemented in
> > > > terms of ExceptionUtils.format but there does not seem to be much to
> gain
> > > > from that.
> > > >
> > > > Gary
> > > >
> > > >
> > > >>
> > > >> On 03/09/2019 19:04, Gary Gregory wrote:
> > > >> > Please read the source.
> > > >> >
> > > >> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com>
> wrote:
> > > >> >
> > > >> >> Why don't you use java.lang.IllegalArgumentException instead?
> > > >> >> And, why we need such a class, but not using
> > > >> >> java.lang.IllegalArgumentException there?
> > > >> >>
> > > >> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> > > >> >>>
> > > >> >>> Hi All:
> > > >> >>>
> > > >> >>> I propose we take
> > > >> >>>
> > > >> >>
> > > >>
> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> > > >> >>> and make it a public class in [lang]. FWIW, I use a class like
> this in
> > > >> >> many
> > > >> >>> projects at work.
> > > >> >>>
> > > >> >>> Gary
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Xeno Amess <xe...@gmail.com>.
No I still don't think it a good idea to have such a util class.
1. most Throwable classes, they all have a constructor with a String.
2. IllegalArgumentException is not special in this way.
3. If we make a public Util class for IllegalArgumentException, then
we shall make similar public Util classes for all other Throwables in
JDK.
However if you really want to add something like that, I think
something like this would be a quite reasonable answer:
import org.junit.jupiter.api.Test;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class ExceptionUtils {
public static <T> T generateException(final Class<T>
exceptionClass, final String format,
final Object... arguments) {
T result = null;
try {
Constructor<T> exceptionConstructor =
exceptionClass.getDeclaredConstructor(String.class);
if (exceptionConstructor != null) {
result =
exceptionConstructor.newInstance(String.format(format, arguments));
}
} catch (NoSuchMethodException | InstantiationException |
IllegalAccessException | InvocationTargetException e) {
throw generateException(IllegalArgumentException.class, e,
"Can't create %s with arguments %s.",
exceptionClass.getCanonicalName(),
Arrays.toString(arguments));
}
return result;
}
public static <T> T generateException(final Class<T>
exceptionClass, Throwable cause, final String format,
final Object... arguments) {
T result = null;
try {
Constructor<T> exceptionConstructor =
exceptionClass.getDeclaredConstructor(String.class, Throwable.class);
if (exceptionConstructor != null) {
result =
exceptionConstructor.newInstance(String.format(format, arguments),
cause);
}
} catch (NoSuchMethodException | InstantiationException |
IllegalAccessException | InvocationTargetException e) {
throw generateException(IllegalArgumentException.class, e,
"Can't create %s with arguments %s.",
exceptionClass.getCanonicalName(),
Arrays.toString(arguments));
}
return result;
}
@Test
public void test() {
RuntimeException re =
generateException(RuntimeException.class, "%d,,%s", 1, "2020");
assertEquals(re.getMessage(), "1,,2020");
RuntimeException re2 =
generateException(RuntimeException.class, re, "%d,,%s", 2, "3030");
assertEquals(re2.getMessage(), "2,,3030");
assertEquals(re2.getCause(), re);
}
}
Gilles Sadowski <gi...@gmail.com> 于2019年9月4日周三 下午7:17写道：
>
> Hi.
>
> Le mer. 4 sept. 2019 à 02:53, Gary Gregory <ga...@gmail.com> a écrit :
> >
> > Here is the PR with [lang]'s own call sites updated to use the new code.
> >
> > Gary
> >
> > On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com> wrote:
> >
> > > On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
> > >
> > >> Why limit this to IllegalArgument? Why not make it more generic? For
> > >> instance, in ExceptionUtils:
> > >>
> > >> public static <T extends Throwable> T format(final Function<? super
> > >> String, ? extends T> factory, final String format, final Object... args) {
> > >> return factory.apply(String.format(format, args));
> > >> }
> > >>
> > >> public static <T extends Throwable> T format(final BiFunction<? super
> > >> String, ? super Throwable, ? extends T> factory, final Throwable t,
> > >> final String format, final Object... args) {
> > >> return factory.apply(String.format(format, args), t);
> > >> }
> > >>
> > >> These can then be called using
> > >> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
> > >> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
> > >> "message: %s", message).
>
> API looks a little bit strange (throw a "format"?)
>
> Perhaps:
> ---CUT---
> throw IllegalArgumentExceptionFactory.withFormat(...);
> ---CUT---
>
> Is there additional customization foreseen (that may require
> a "builder")?
>
> Also, wouldn't it be useful to not perform the formatting
> unconditionally but rather when "getMessage() is called?
>
> Gilles
>
> > >>
> > >> It's a bit verbose though, but it gives a lot more flexibility.
> > >>
> > >
> > > Yes, we could add these to ExceptionUtils separately IMO; but, the
> > > verbosity is an issue for me. This is straightforward:
> > >
> > > throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
> > > source, method, pathSpec, operation.getOperationId(), e.toString());
> > >
> > > The following not as much:
> > >
> > > throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
> > > operation %s: %s", source, method, pathSpec, operation.getOperationId(),
> > > e.toString());
> > >
> > > So it could be that IllegalArgumentExceptions.format() is implemented in
> > > terms of ExceptionUtils.format but there does not seem to be much to gain
> > > from that.
> > >
> > > Gary
> > >
> > >
> > >>
> > >> On 03/09/2019 19:04, Gary Gregory wrote:
> > >> > Please read the source.
> > >> >
> > >> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
> > >> >
> > >> >> Why don't you use java.lang.IllegalArgumentException instead?
> > >> >> And, why we need such a class, but not using
> > >> >> java.lang.IllegalArgumentException there?
> > >> >>
> > >> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> > >> >>>
> > >> >>> Hi All:
> > >> >>>
> > >> >>> I propose we take
> > >> >>>
> > >> >>
> > >> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> > >> >>> and make it a public class in [lang]. FWIW, I use a class like this in
> > >> >> many
> > >> >>> projects at work.
> > >> >>>
> > >> >>> Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gilles Sadowski <gi...@gmail.com>.
Hi.
Le mer. 4 sept. 2019 à 02:53, Gary Gregory <ga...@gmail.com> a écrit :
>
> Here is the PR with [lang]'s own call sites updated to use the new code.
>
> Gary
>
> On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com> wrote:
>
> > On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
> >
> >> Why limit this to IllegalArgument? Why not make it more generic? For
> >> instance, in ExceptionUtils:
> >>
> >> public static <T extends Throwable> T format(final Function<? super
> >> String, ? extends T> factory, final String format, final Object... args) {
> >> return factory.apply(String.format(format, args));
> >> }
> >>
> >> public static <T extends Throwable> T format(final BiFunction<? super
> >> String, ? super Throwable, ? extends T> factory, final Throwable t,
> >> final String format, final Object... args) {
> >> return factory.apply(String.format(format, args), t);
> >> }
> >>
> >> These can then be called using
> >> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
> >> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
> >> "message: %s", message).
API looks a little bit strange (throw a "format"?)
Perhaps:
---CUT---
throw IllegalArgumentExceptionFactory.withFormat(...);
---CUT---
Is there additional customization foreseen (that may require
a "builder")?
Also, wouldn't it be useful to not perform the formatting
unconditionally but rather when "getMessage() is called?
Gilles
> >>
> >> It's a bit verbose though, but it gives a lot more flexibility.
> >>
> >
> > Yes, we could add these to ExceptionUtils separately IMO; but, the
> > verbosity is an issue for me. This is straightforward:
> >
> > throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
> > source, method, pathSpec, operation.getOperationId(), e.toString());
> >
> > The following not as much:
> >
> > throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
> > operation %s: %s", source, method, pathSpec, operation.getOperationId(),
> > e.toString());
> >
> > So it could be that IllegalArgumentExceptions.format() is implemented in
> > terms of ExceptionUtils.format but there does not seem to be much to gain
> > from that.
> >
> > Gary
> >
> >
> >>
> >> On 03/09/2019 19:04, Gary Gregory wrote:
> >> > Please read the source.
> >> >
> >> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
> >> >
> >> >> Why don't you use java.lang.IllegalArgumentException instead?
> >> >> And, why we need such a class, but not using
> >> >> java.lang.IllegalArgumentException there?
> >> >>
> >> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> >> >>>
> >> >>> Hi All:
> >> >>>
> >> >>> I propose we take
> >> >>>
> >> >>
> >> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> >> >>> and make it a public class in [lang]. FWIW, I use a class like this in
> >> >> many
> >> >>> projects at work.
> >> >>>
> >> >>> Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Sep 3, 2019 at 8:53 PM Gary Gregory <ga...@gmail.com> wrote:
> Here is the PR with [lang]'s own call sites updated to use the new code:
>
https://github.com/apache/commons-lang/pull/450
Gary
>
> Gary
>
> On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com>
> wrote:
>
>> On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
>>
>>> Why limit this to IllegalArgument? Why not make it more generic? For
>>> instance, in ExceptionUtils:
>>>
>>> public static <T extends Throwable> T format(final Function<? super
>>> String, ? extends T> factory, final String format, final Object... args)
>>> {
>>> return factory.apply(String.format(format, args));
>>> }
>>>
>>> public static <T extends Throwable> T format(final BiFunction<? super
>>> String, ? super Throwable, ? extends T> factory, final Throwable t,
>>> final String format, final Object... args) {
>>> return factory.apply(String.format(format, args), t);
>>> }
>>>
>>> These can then be called using
>>> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
>>> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
>>> "message: %s", message).
>>>
>>> It's a bit verbose though, but it gives a lot more flexibility.
>>>
>>
>> Yes, we could add these to ExceptionUtils separately IMO; but, the
>> verbosity is an issue for me. This is straightforward:
>>
>> throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
>> source, method, pathSpec, operation.getOperationId(), e.toString());
>>
>> The following not as much:
>>
>> throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
>> operation %s: %s", source, method, pathSpec, operation.getOperationId(),
>> e.toString());
>>
>> So it could be that IllegalArgumentExceptions.format() is implemented in
>> terms of ExceptionUtils.format but there does not seem to be much to gain
>> from that.
>>
>> Gary
>>
>>
>>>
>>> On 03/09/2019 19:04, Gary Gregory wrote:
>>> > Please read the source.
>>> >
>>> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
>>> >
>>> >> Why don't you use java.lang.IllegalArgumentException instead?
>>> >> And, why we need such a class, but not using
>>> >> java.lang.IllegalArgumentException there?
>>> >>
>>> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
>>> >>>
>>> >>> Hi All:
>>> >>>
>>> >>> I propose we take
>>> >>>
>>> >>
>>> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
>>> >>> and make it a public class in [lang]. FWIW, I use a class like this
>>> in
>>> >> many
>>> >>> projects at work.
>>> >>>
>>> >>> Gary
>>> >>
>>> >> ---------------------------------------------------------------------
>>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> >> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
Here is the PR with [lang]'s own call sites updated to use the new code.
Gary
On Tue, Sep 3, 2019 at 2:13 PM Gary Gregory <ga...@gmail.com> wrote:
> On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
>
>> Why limit this to IllegalArgument? Why not make it more generic? For
>> instance, in ExceptionUtils:
>>
>> public static <T extends Throwable> T format(final Function<? super
>> String, ? extends T> factory, final String format, final Object... args) {
>> return factory.apply(String.format(format, args));
>> }
>>
>> public static <T extends Throwable> T format(final BiFunction<? super
>> String, ? super Throwable, ? extends T> factory, final Throwable t,
>> final String format, final Object... args) {
>> return factory.apply(String.format(format, args), t);
>> }
>>
>> These can then be called using
>> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
>> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
>> "message: %s", message).
>>
>> It's a bit verbose though, but it gives a lot more flexibility.
>>
>
> Yes, we could add these to ExceptionUtils separately IMO; but, the
> verbosity is an issue for me. This is straightforward:
>
> throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
> source, method, pathSpec, operation.getOperationId(), e.toString());
>
> The following not as much:
>
> throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
> operation %s: %s", source, method, pathSpec, operation.getOperationId(),
> e.toString());
>
> So it could be that IllegalArgumentExceptions.format() is implemented in
> terms of ExceptionUtils.format but there does not seem to be much to gain
> from that.
>
> Gary
>
>
>>
>> On 03/09/2019 19:04, Gary Gregory wrote:
>> > Please read the source.
>> >
>> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
>> >
>> >> Why don't you use java.lang.IllegalArgumentException instead?
>> >> And, why we need such a class, but not using
>> >> java.lang.IllegalArgumentException there?
>> >>
>> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
>> >>>
>> >>> Hi All:
>> >>>
>> >>> I propose we take
>> >>>
>> >>
>> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
>> >>> and make it a public class in [lang]. FWIW, I use a class like this in
>> >> many
>> >>> projects at work.
>> >>>
>> >>> Gary
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >> For additional commands, e-mail: dev-help@commons.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <ap...@icemanx.nl> wrote:
> Why limit this to IllegalArgument? Why not make it more generic? For
> instance, in ExceptionUtils:
>
> public static <T extends Throwable> T format(final Function<? super
> String, ? extends T> factory, final String format, final Object... args) {
> return factory.apply(String.format(format, args));
> }
>
> public static <T extends Throwable> T format(final BiFunction<? super
> String, ? super Throwable, ? extends T> factory, final Throwable t,
> final String format, final Object... args) {
> return factory.apply(String.format(format, args), t);
> }
>
> These can then be called using
> ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
> message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
> "message: %s", message).
>
> It's a bit verbose though, but it gives a lot more flexibility.
>
Yes, we could add these to ExceptionUtils separately IMO; but, the
verbosity is an issue for me. This is straightforward:
throw IllegalArgumentExceptions.format(e, "%s: %s %s operation %s: %s",
source, method, pathSpec, operation.getOperationId(), e.toString());
The following not as much:
throw ExceptionUtils.format(IllegalArgumentException::new, e, "%s: %s %s
operation %s: %s", source, method, pathSpec, operation.getOperationId(),
e.toString());
So it could be that IllegalArgumentExceptions.format() is implemented in
terms of ExceptionUtils.format but there does not seem to be much to gain
from that.
Gary
>
> On 03/09/2019 19:04, Gary Gregory wrote:
> > Please read the source.
> >
> > On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
> >
> >> Why don't you use java.lang.IllegalArgumentException instead?
> >> And, why we need such a class, but not using
> >> java.lang.IllegalArgumentException there?
> >>
> >> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> >>>
> >>> Hi All:
> >>>
> >>> I propose we take
> >>>
> >>
> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> >>> and make it a public class in [lang]. FWIW, I use a class like this in
> >> many
> >>> projects at work.
> >>>
> >>> Gary
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Rob Spoor <ap...@icemanx.nl>.
Why limit this to IllegalArgument? Why not make it more generic? For
instance, in ExceptionUtils:
public static <T extends Throwable> T format(final Function<? super
String, ? extends T> factory, final String format, final Object... args) {
return factory.apply(String.format(format, args));
}
public static <T extends Throwable> T format(final BiFunction<? super
String, ? super Throwable, ? extends T> factory, final Throwable t,
final String format, final Object... args) {
return factory.apply(String.format(format, args), t);
}
These can then be called using
ExceptionUtils.format(IllegalArgumentException::new, "message: %s",
message) or ExceptionUtils.format(IllegalArgumentException::new, cause,
"message: %s", message).
It's a bit verbose though, but it gives a lot more flexibility.
On 03/09/2019 19:04, Gary Gregory wrote:
> Please read the source.
>
> On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
>
>> Why don't you use java.lang.IllegalArgumentException instead?
>> And, why we need such a class, but not using
>> java.lang.IllegalArgumentException there?
>>
>> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
>>>
>>> Hi All:
>>>
>>> I propose we take
>>>
>> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
>>> and make it a public class in [lang]. FWIW, I use a class like this in
>> many
>>> projects at work.
>>>
>>> Gary
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Gary Gregory <ga...@gmail.com>.
Please read the source.
On Tue, Sep 3, 2019, 12:27 Xeno Amess <xe...@gmail.com> wrote:
> Why don't you use java.lang.IllegalArgumentException instead?
> And, why we need such a class, but not using
> java.lang.IllegalArgumentException there?
>
> Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
> >
> > Hi All:
> >
> > I propose we take
> >
> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> > and make it a public class in [lang]. FWIW, I use a class like this in
> many
> > projects at work.
> >
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.text.lookup.IllegalArgumentExceptions
to [lang].
Posted by Xeno Amess <xe...@gmail.com>.
Why don't you use java.lang.IllegalArgumentException instead?
And, why we need such a class, but not using
java.lang.IllegalArgumentException there?
Gary Gregory <ga...@gmail.com> 于2019年9月3日周二 下午11:18写道：
>
> Hi All:
>
> I propose we take
> https://commons.apache.org/proper/commons-text/xref/org/apache/commons/text/lookup/IllegalArgumentExceptions.html#IllegalArgumentExceptions
> and make it a public class in [lang]. FWIW, I use a class like this in many
> projects at work.
>
> Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org