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 Wed, 04 Sep 2019 14:22:20 GMT
Le mer. 4 sept. 2019 à 14:34, Xeno Amess <xenoamess@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 <gilleseran@gmail.com> 于2019年9月4日周三 下午7:17写道:
> >
> > Hi.
> >
> > Le mer. 4 sept. 2019 à 02:53, Gary Gregory <garydgregory@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 <garydgregory@gmail.com>
wrote:
> > >
> > > > On Tue, Sep 3, 2019 at 1:19 PM Rob Spoor <apache@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 <xenoamess@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 <garydgregory@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


Mime
View raw message