And what I've done with PatternLayout.Builder is use null by default, and in the build() method, I check if it's still null. If it is, I construct a DefaultConfiguration to use instead. In the DefaultConfiguration constructor, I pass "this" as the configuration to PatternLayout.Builder to avoid creating an infinite recursion loop.
On 25 May 2014 22:46, Matt Sicker <[email protected]> wrote: > The Builders are simpler to use than factory methods that have tons of > parameters (many of which we tend to rely on defaults in unit tests). It > will also allow us to use classes and field injection instead of parameter > injection for creating plugins. It will also pave the way to allowing > programmatic configuration of Log4j via a Java DSL (which may end up being > mostly useful for unit tests). It's also easier to specify default values > for typed attributes when you can use the built-in Java support for that > instead of converting strings. > > > On 25 May 2014 22:42, Remko Popma <[email protected]> wrote: > >> The Builder! Now I finally get what you're trying to achieve. I didn't >> get that at all from your previous description! >> >> So this has nothing to do with headers/footers then, right? Wow, that >> really threw me off the track. >> >> So, in order to accomplish PatternLayout.Builder, you propose to change >> the DefaultConfiguration constructor to pass a NullConfig to >> PatternLayout.createLayout, is that correct? >> >> I don't have a problem with that, as long as it doesn't break anything >> (don't see how it could, as it is currently null, but anyway). >> >> The Builders are beginning to sound pretty complex though. Why do we need >> this again? >> >> Sent from my iPhone >> >> On 2014/05/26, at 11:58, Matt Sicker <[email protected]> wrote: >> >> I was hitting it because I wasn't passing "this". I was using >> DefaultConfiguration as the default field value for config in the >> PatternLayout.Builder class. >> >> >> On 25 May 2014 21:46, Remko Popma <[email protected]> wrote: >> >>> I meant: ...where you were hitting the constructor loop you mentioned >>> earlier. >>> >>> >>> On Monday, May 26, 2014, Remko Popma <[email protected]> wrote: >>> >>>> I see, so you need this for property substitution in the header/footer. >>>> In that case you probably need the real config. If it works to pass "this" >>>> when creating the layout in the DefaultConfiguration constructor, then >>>> great. (I just assumed that this is where you hit the constructor eternal >>>> loop.) >>>> >>>> >>>> Sent from my iPhone >>>> >>>> On 2014/05/26, at 11:27, Matt Sicker <[email protected]> wrote: >>>> >>>> Well, it looks like the only place config is used is if header or >>>> footer are non-null. There it is used in order to get its StrSubstitutor. >>>> Otherwise, config is always checked for null. So DefaultConfiguration could >>>> probably use NullConfiguration, or it could even use null. >>>> >>>> Either way, wouldn't it work to use "this" during the >>>> DefaultConfiguration constructor? That would make the PatternLayout it's >>>> creating have the most correct configuration set. >>>> >>>> >>>> On 25 May 2014 21:03, Remko Popma <[email protected]> wrote: >>>> >>>> Ok, so until now, DefaultConfiguration uses a PatternLayout with a null >>>> Configuration. But now you're considering a change that requires that >>>> PatternLayout has a non-null Configuration. >>>> >>>> I guess it depends on how you intend to use the PatternLayout's >>>> Configuration. If all that's needed is for it to be non-null, we could pass >>>> a NullConfiguration to PatternLayout.createLayout in the >>>> DefaultConfiguration constructor (passing "this" won't work). >>>> Is that enough, or does the PatternLayout's Configuration need to be >>>> the "real" config? >>>> >>>> That's why I'm asking why you need this. >>>> >>>> Sent from my iPhone >>>> >>>> On 2014/05/26, at 10:13, Matt Sicker <[email protected]> wrote: >>>> >>>> DefaultConfiguration needs a Layout, and it uses PatternLayout for >>>> that. In PatternLayout, I don't want its Configuration to be null due to >>>> the field being used elsewhere by PatternLayout. Thus, to include a >>>> DefaultConfiguration in PatternLayout, and then a PatternLayout in >>>> DefaultConfiguration, I have to leak the this reference during construction >>>> to avoid creating an infinite loop of constructors. >>>> >>>> >>>> On 25 May 2014 20:10, Remko Popma <[email protected]> wrote: >>>> >>>> I had a look at the code and it may not be easy to make changes. Can we >>>> go back a step and can you explain what you're trying to achieve? (What do >>>> you want to change & why?) >>>> >>>> Sent from my iPhone >>>> >>>> On 2014/05/26, at 9:08, Matt Sicker <[email protected]> wrote: >>>> >>>> Actually, that still leaves the problem of specifying a Configuration >>>> for the DefaultConfigurationLayout. PatternLayout is final right now, and >>>> the default pattern is "%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - >>>> %msg%n". >>>> >>>> >>>> On 25 May 2014 19:03, Matt Sicker <[email protected]> wrote: >>>> >>>> That sounds like a better idea. Let me see how well it works out in >>>> code. >>>> >>>> >>>> On 25 May 2014 18:52, Remko Popma <[email protected]> wrote: >>>> >>>> Perhaps DefaultConfigurationLayout would be a better name for such >>>> alayout, actually. >>>> >>>> >>>> On Monday, May 26, 2014, Remko Popma <[email protected]> wrote: >>>> >>>> How about having a DefaultLayout for use *only* by >>>> DefaultConfiguration? The formatters used by this layout can be hard-coded: >>>> level, timestamp, message. >>>> Thoughts? >>>> >>>> Sent from my iPhone >>>> >>>> On 2014/05/26, at 8:38, Matt Sicker <[email protected]> wrote: >>>> >>>> >> >> >> -- >> Matt Sicker <[email protected]> >> >> > > > -- > Matt Sicker <[email protected]> > -- Matt Sicker <[email protected]>
