What are the arguments here and why would it be from() rather than the fairly common of()?
from() generally suggests you convert one thing to another with the same underlying semantics. LocalDate.from() is an example Obtains an instance of LocalDate from a temporal object. Everywhere else that API usually calls it of(), too. The only exception in several places (not just JDK or JSRs) would be parse() if the argument is strictly a string or textual format. If it's a complete no-op, create(), getInstance() or instance() would be good alternatives, for those with arguments, see above. Werner On Thu, Mar 19, 2015 at 2:08 PM, Romain Manni-Bucau <[email protected]> wrote: > why not from(xxx) directly? In any case your proposals sound better for the > frenchy I am than of(xxx) ;). > > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <http://rmannibucau.wordpress.com> | Github < > https://github.com/rmannibucau> | > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber > <http://www.tomitribe.com> > > 2015-03-19 13:40 GMT+01:00 Tresch, Anatole < > [email protected] > >: > > > I, saw, still an answer is required here ;) > > > > -----Original Message----- > > From: Romain Manni-Bucau [mailto:[email protected]] > > Sent: Montag, 16. März 2015 19:32 > > To: [email protected] > > Subject: Re: ConfigurationBuilder > > > > 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. > > > > -> How about create(), createFromCurrentConfig(), > > createFrom(Configuration) ? > > > > > > > > *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 > > > > -> I personally always find it confusing when its singular, but you can > > multiple ones, whereas the other way round > > a single one mathematically speaking can be seen as a subset of the > > multiple ones, which the other way round is > > not the case, so I prefer having the 's'... > > > > > > > > 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? > > -> 1:0 for you. Keep it simple... > > > > > > > > > > *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) > > > > -> Matter of taste IMO. I will not insist on this point... > > > > > > > > > > *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 > > -> As above: I will not insist on this point... > > > > > > 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* > > > > > > > > > >
