Well about optim I think that is one of the place we care the less since
you'll use it not often and I am not even sure you can notice it at all.
Keeping API simpler is better to start.
Le 17 mars 2015 08:31, "Oliver B. Fischer" <[email protected]> a
écrit :
> Hi,
>
> where is the advantage of a factory method if the result of this method is
> always of the same type as the class providing the factory method? Static
> factory methods look cool and modern but I do not like them as there is no
> advantage at the moment in using them for this class.
>
> Concerning adding new items. Providing these overloaded method helps the
> compiler to optimize the generated code as it selects the best fit for the
> method call and helps to avoid temporary objects.
>
> Bye,
>
> Oliver
>
>
>
> Am 16.03.15 um 13:55 schrieb Anatole Tresch:
>
>> 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
>>
>> *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).
>>
>> Given that I would propose the signature to be looking as:
>>
>> a) ConfigurationBuilder addItems(Item... items);
>> b) ConfigurationBuilder addItems(Collection<Item> items);
>>
>>
>> *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.
>>
>>
>> *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.
>>
>> So far my feedback.Let me know what you think!
>>
>> Have a nice day!
>> Anatole
>>
>>
>>
> --
> N Oliver B. Fischer
> A Schönhauser Allee 64, 10437 Berlin, Deutschland/Germany
> P +49 30 44793251
> M +49 178 7903538
> E [email protected]
> S oliver.b.fischer
> J [email protected]
> X http://xing.to/obf
>
>
