I thought of an interesting work around in unit tests by using the
PluginBuilder directly by setting up a Node object manually. This seemed
more clear because it requires using the named parameters. I could probably
work on a fluent interface to using that.
Also, we use the factory methods in production code, not just tests.
On Monday, 16 June 2014, Remko Popma <remko.po...@gmail.com> wrote:
> (I'm talking about doing JUnit tests on the factory methods in case that
> was not clear.)
>
>
> On Mon, Jun 16, 2014 at 9:42 PM, Remko Popma <remko.po...@gmail.com>
> wrote:
>
> ...and another (shorter) way would be to add a comment for each parameter:
> factoryMethod(
> null, // loggerName
> null, // Marker
> null, // LoggerFQCN
> null, // Level
> null, // Message
> null, // Throwable
> null, // ContextMap
> null, // ContextStack
> null, // ThreadName
> null, // Location
> 0); // CurrentTime
>
>
>
> On Mon, Jun 16, 2014 at 9:36 PM, Remko Popma <remko.po...@gmail.com>
> wrote:
>
> One very simple way I use to clarify which parameter is what is to declare
> a local variable for each param.
> For example:
>
> (and this is not strictly a @Plugin factory method, but you get my
> point...)
>
> public class RingBufferLogEventTest {
> @Test
> public void testGetLevelReturnsOffIfNullLevelSet() {
> RingBufferLogEvent evt = new RingBufferLogEvent();
> String loggerName = null;
> Marker marker = null;
> String fqcn = null;
> Level level = null;
> Message data = null;
> Throwable t = null;
> Map<String, String> map = null;
> ContextStack contextStack = null;
> String threadName = null;
> StackTraceElement location = null;
> long currentTimeMillis = 0;
> evt.setValues(null, loggerName, marker, fqcn, level, data, t, map,
> contextStack, threadName, location, currentTimeMillis);
> assertEquals(Level.OFF, evt.getLevel());
> }
>
> To me this is just as clear as
>
> builder.setLoggerName(null)
> .withMarker(null)
> .withLoggerFQCN(null)
> .withLevel(null)
> .withMessage(null)
> .withThrowable(null)
> .withContextMap(null)
> .withContextStack(null)
> .withThreadName(null)
> .withLocation(null)
> .withCurrentTime(0)
> .build()
>
>
> On Mon, Jun 16, 2014 at 1:10 PM, Matt Sicker <boa...@gmail.com> wrote:
>
> Well, I can live with factory methods if we have a better method for doing
> unit tests. Not all unit tests need an XML configuration file.
>
>
> On 15 June 2014 22:04, 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 Sick
>
>
--
Matt Sicker <boa...@gmail.com>
