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
