2015-03-16 14:10 GMT+01:00 Werner Keil <[email protected]>:
> Hi,
>
> The idea of a factory sounds good. I don't like the empty of() since it
> goes against all the current APIs where of() is used, see Java SE 8
> Date/Time, there is always something like of(argument) or ofMillis(int),
> etc.
> The only notable no-op factory methods are things like now().
>
> So current() (also in CDI) seems understandable, ofCurrent() disturbing,
> but the idea OF having such static factory methods seems OK to me;-)
>
> Werner
>
>
>
> On Mon, Mar 16, 2015 at 1:55 PM, Anatole Tresch <[email protected]>
> wrote:
>
> > Hi all
> >
> > I have looked at the new builder module a bit more deeper and I just
> write
> > down my suggestions here, so we can discuss them. Some of them also might
> > be of general interest, regarding how APIs should be built in other
> modules
> > as well:
> >
> >
> > *Constructing a new ConfigurationBuilder:*
> >
> > Currently a user must type new ConfigurationBuilder(); to create a new
> > builder. How about using static factory methods for doing this (in JSR
> 354
> > user feedback, e.g. from LJC was doing it that way would be preferred).
> As
> > an example, we could define the following:
> >
> > ConfigurationBuilder.of(); // creating a new empty instance
> > ConfigurationBuilder.ofCurrent(); // create a new builder, using the
> > // current configuration as a
> > // starting point, allowing to
> easily
> > // adapt an existing
> configuration...
> > ConfigurationBuilder of(Configuration config); // create a new builder,
> > // using the given
> > // configuration
> >
>
+-0, using an inner class is as common as it AFAIK. That said I hate "of"
for builder, doesnt mean anything for me.
> > *Adding new "items":*
> >
> > There are locations where methods are designed as follows:
> >
> > a) ConfigurationBuilder addItem(Item item, Item... items);
> > b) ConfigurationBuilder addItem(Item item);
> >
> > e.g. ConfigurationBuilder addPropertySource(...).
> >
> > -> Basically the API should be minimalistic, so
> > 1) I assume we can omit method b in that case.
> > 2) Collections (or Iterable) are not supported, so I would think of
> > adding Collection support as
> > well.
> > 2) Since multiple instances could be passed, I would suggest to name
> > add an 's' to its name.
> > 3) In "Effective Java" the variant with the single item in front is
> > mentioned as an example,
> > where at least one instance must be passed. I personally think
> this
> > variant is
> > overengineered, since we can still either check the input, or
> > simply return (optionally
> > logging a warning).
> >
>
Why not simply addItems(Item...items) - with or without the "s" I don't
care much
> > Given that I would propose the signature to be looking as:
> >
> > a) ConfigurationBuilder addItems(Item... items);
> > b) ConfigurationBuilder addItems(Collection<Item> items);
> >
>
-1 to have both - sounds opposed to what you just said, no?
> >
> > *Boolean Property Signatures:*
> >
> > public boolean isPropertyConverterLoadingEnabled() {...}
> > public ConfigurationBuilder enableProvidedPropertyConverters() {...}
> > public ConfigurationBuilder disableProvidedPropertyConverters() {...}
> >
> > There are multiple boolean flags in the builder (the example ebove shows
> > one of them).
> > I personally would expect the Java properties style to be applied here,
> so
> > it should be changed to:
> >
> > public boolean isPropertyConverterLoadingEnabled() {...}
> > public ConfigurationBuilder setProvidedPropertyConverters(boolean
> enabled)
> > {...}
> >
> > This reduces the API by 1 method.
> >
>
For a builder on/off flag methods are common and more expressive. If we go
to setters I'd use setters eveywhere (see my other thread on this)
> >
> > *StringToURLMapper*
> >
> > I would suggest implementing this as a Lambda expression avoiding the
> Java
> > class overhead. If compatibility was the focus with Java 7 it should not
> > extend java.util.function.Function, because this class is not available
> in
> > Java 7.
> >
>
+-0 overhead = 0 since lambda implies the same class generation by the JVM
> > So far my feedback.Let me know what you think!
> >
> > Have a nice day!
> > Anatole
> >
> >
> > --
> > *Anatole Tresch*
> > Java Engineer & Architect, JSR Spec Lead
> > Glärnischweg 10
> > CH - 8620 Wetzikon
> >
> > *Switzerland, Europe Zurich, GMT+1*
> > *Twitter: @atsticks*
> > *Blogs: **http://javaremarkables.blogspot.ch/
> > <http://javaremarkables.blogspot.ch/>*
> >
> > *Google: atsticksMobile +41-76 344 62 79*
> >
>
