Programmatic configuration is feature in that list of issues Ralph
mentioned. I don't see any good way to do that without using the fluent
builder pattern as that's how programmatic configuration APIs are usually
done. The other ways are usually things like closures/lambdas, but that
would require Java 1.8+ to work. Groovy supports closures which is what
makes Gradle so much less verbose, so that, too, is an alternative way to
support builders.
I guess the important thing to consider here is that the builders aren't
for _plugins_, but are for plugin _configurations_. That makes me think
that the factory methods (whether they be private or public) should be far
more generic such as accepting a PluginConfiguration object (each specific
to the plugin) or a generic Map<String, Object> that also makes it a lot
simpler to pass in plugin attributes.
On 16 June 2014 10:05, Gary Gregory <garydgreg...@gmail.com> wrote:
> Right, so they are not part of the public API, unless we consider
> programmatic configuration (not from a file).
>
> Gary
>
>
> On Mon, Jun 16, 2014 at 10:56 AM, Remko Popma <remko.po...@gmail.com>
> wrote:
>
>> Paul this is about the plugin factory methods that are called by the
>> PluginManager to turn a configuration text file into an object hierarchy
>> with Loggers, Appenders, etc.
>> It would be very rare for client code to call these methods directly.
>> Which is why they are primarily called from JUnit tests.
>>
>>
>> On Mon, Jun 16, 2014 at 11:45 PM, Paul Benedict <pbened...@apache.org>
>> wrote:
>>
>>> If this API is primarily for unit tests, then I don't care as much. I
>>> thought this was Public API. When it comes to Public API, I prefer to make
>>> it as friendly-to-read and maintainable as possible.
>>>
>>>
>>> Cheers,
>>> Paul
>>>
>>>
>>> On Mon, Jun 16, 2014 at 9:42 AM, Remko Popma <remko.po...@gmail.com>
>>> wrote:
>>>
>>>> About the validation, wasn't the argument that some of the current
>>>> JUnit tests look like this?
>>>> FileAppender.createAppender("true", "true", "true", "true", "true",
>>>> "true", "true", "4096", null, null, "false", null, null);
>>>>
>>>> So, null is fine here for at least four arguments.
>>>> So it is definitely possible to forget to call the
>>>> builder.setValue(value) method for those arguments
>>>> - which is fine if your intention was to set null, but not if you
>>>> forgot to set some non-null value.
>>>>
>>>> The compiler validates that you have the right number of arguments for
>>>> free.
>>>> It is not possible for builders to provide the same guarantee unless
>>>> all arguments must be non-null.
>>>>
>>>> About the readability - and as Ralph pointed out this is readability in
>>>> JUnit code primarily - there are a number of ways to accomplish that.
>>>>
>>>>
>>>>
>>>> On Mon, Jun 16, 2014 at 11:27 PM, Ralph Goers <
>>>> ralph.go...@dslextreme.com> wrote:
>>>>
>>>>> That is pretty much what Matt implemented. See the other arguments
>>>>> around this, the primary one being that the factory methods are normally
>>>>> invoked dynamically so you wouldn’t be looking at the code that calls
>>>>> them.
>>>>> They are primarily called in the open in unit tests.
>>>>>
>>>>> Ralph
>>>>>
>>>>> On Jun 16, 2014, at 6:49 AM, Paul Benedict <pbened...@apache.org>
>>>>> wrote:
>>>>>
>>>>> I don't know why the Spring link isn't working (try later) but this is
>>>>> how I think you guys can make the builder pattern better. Honestly, it's
>>>>> impossible to comprehend what the parameters mean just by looking at the
>>>>> code in the current form.
>>>>>
>>>>> Example returning a custom builder interface:
>>>>> FileAppender.builder()
>>>>> .setPropertyX(true)
>>>>> .setPropertyY(true)
>>>>> .setPropertyZ(4096)
>>>>> .setPropertyA(null)
>>>>> .create();
>>>>>
>>>>> Paul
>>>>>
>>>>>
>>>>>
>>>>> Cheers,
>>>>> Paul
>>>>>
>>>>>
>>>>> On Mon, Jun 16, 2014 at 8:41 AM, Remko Popma <remko.po...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Matt,
>>>>>>
>>>>>> Ralph's argument (and I agree) was that we want only one way to do
>>>>>> plugins, either with a factory method or with a builder.
>>>>>>
>>>>>> Currently only two plugins use the new builder: PatternLayout and
>>>>>> HtmlLayout.
>>>>>> So we can either revert these two back to use a factory method, or we
>>>>>> can convert the remaining 147 uses of the factory method to builders.
>>>>>> That last option would mean writing the builders for those plugins,
>>>>>> and modifying all JUnit tests that currently use the factory methods.
>>>>>>
>>>>>> I agree with Ralph that converting all remaining 147 places to use
>>>>>> builders would take up a lot of our time and energy for very little gain.
>>>>>>
>>>>>> Matt, you mentioned that you could live with factory methods if there
>>>>>> was a better way to write JUnit tests than
>>>>>> FileAppender.createAppender("true", "true", "true", "true", "true",
>>>>>> "true", "true", "4096", null, null, "false", null, null);
>>>>>>
>>>>>> I believe I've demonstrated several ways in which such JUnit test
>>>>>> code can be improved.
>>>>>> Are you okay with removing the builders and reverting back to the
>>>>>> factory methods?
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 16, 2014 at 10:23 PM, Matt Sicker <boa...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I'm unable to load that page for some reason, but yes, using a
>>>>>>> factory method as such is really not extensible. If the factory method
>>>>>>> took
>>>>>>> a Map or some sort of configuration object (which itself could use the
>>>>>>> fluent builder pattern), that would be less tightly coupled.
>>>>>>>
>>>>>>> Wouldn't it also make sense for the factory methods to be private or
>>>>>>> similar? That way they're only accessible through reflection.
>>>>>>>
>>>>>>>
>>>>>>> On Sunday, 15 June 2014, Paul Benedict <pbened...@apache.org> wrote:
>>>>>>>
>>>>>>>> You don't want factory methods that take umpteen arguments. That's
>>>>>>>> no way to make your builder extensible for future enhancements.
>>>>>>>> Instead,
>>>>>>>> you want a builder object that has a fluent API.
>>>>>>>>
>>>>>>>>
>>>>>>>> http://docs.spring.io/spring/docs/3.2.8.RELEASE/javadoc-api/org/springframework/beans/factory/support/BeanDefinitionBuilder.html
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Paul
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sun, Jun 15, 2014 at 10:04 PM, Ralph Goers <
>>>>>>>> ralph.go...@dslextreme.com> wrote:
>>>>>>>>
>>>>>>>> Matt,
>>>>>>>>
>>>>>>>> The only objection I have to builders is that there should only be
>>>>>>>> one way to configure plugins and I have neither the time or energy to
>>>>>>>> convert all plugins from factories to builders. With 130+ open issues I
>>>>>>>> think our time is better focused there instead of fixing something that
>>>>>>>> already works.
>>>>>>>>
>>>>>>>> And, FWIW, the only place you will see createAppender coded like
>>>>>>>> that is in unit tests and there are a bunch of ways to make that
>>>>>>>> clearer.
>>>>>>>>
>>>>>>>> Ralph
>>>>>>>>
>>>>>>>> On Jun 15, 2014, at 7:19 PM, Matt Sicker <boa...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> I'm really against using factory methods due to language
>>>>>>>> limitations in Java. You can't specify default values, for one. Two,
>>>>>>>> the
>>>>>>>> more parameters a factory takes, the crazier the method is. Seriously,
>>>>>>>> tell
>>>>>>>> me what this method is specifying:
>>>>>>>>
>>>>>>>> FileAppender.createAppender("true", "true", "true", "true", "true",
>>>>>>>> "true", "true", "4096", null, null, "false", null, null);
>>>>>>>>
>>>>>>>>
>>>>>>>> On 15 June 2014 21:05, Remko Popma <remko.po...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> I'm fine with just the factory methods too.
>>>>>>>>
>>>>>>>> Sent from my iPhone
>>>>>>>>
>>>>>>>> On 2014/06/16, at 9:44, Scott Deboy <scott.de...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> +1
>>>>>>>> On Jun 15, 2014 4:05 PM, "Ralph Goers" <ralph.go...@dslextreme.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Do we need the builders? As I said, I prefer only one way for
>>>>>>>> creating plugins.
>>>>>>>>
>>>>>>>> Ralph
>>>>>>>>
>>>>>>>> On Jun 15, 2014, at 2:49 PM, Remko Popma <remko.po...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> I see. I agree that the original format is much nicer.
>>>>>>>>
>>>>>>>> Matt, do you think you can achieve this with the builders?
>>>>>>>>
>>>>>>>> Sent from my iPhone
>>>>>>>>
>>>>>>>> On 2014/06/16, at 1:29, Ralph Goers <ralph.go...@dslextreme.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> While you improved some of the existing messages, you really didm’t
>>>>>>>> address what I wanted fixed. The previous debug logs would have had
>>>>>>>> messages similar to:
>>>>>>>>
>>>>>>>> Calling createLayout on class
>>>>>>>> org.apache.logging.log4j.core.layout.PatternLayout for element
>>>>>>>> PatternLayout with params(pattern="%d{HH:mm:ss.SSS} [%t] %-5level
>>>>>>>> %logger{36} - %msg%n",
>>>>>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>>>>>> null, charset="null", alwaysWriteExceptions="null")
>>>>>>>>
>>>>>>>> Calling createAppender on class
>>>>>>>> org.apache.logging.log4j.core.appender.ConsoleAppender for element
>>>>>>>> Console
>>>>>>>> with params(PatternLayout(%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} -
>>>>>>>> %msg%n), null, target="SYSTEM_OUT", name="console", follow="null",
>>>>>>>> ignoreExceptions="null")
>>>>>>>>
>>>>>>>> Calling createAppenderRef on class
>>>>>>>> org.apache.logging.log4j.core.config.AppenderRef for element
>>>>>>>> appender-ref
>>>>>>>> with params(ref="console", level="null", null)
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <boa...@gmail.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>
> --
> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
--
Matt Sicker <boa...@gmail.com>
