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: >>>>> >>>>> So DefaultConfiguration creates a PatternLayout, but PatternLayout >>>>> takes a Configuration. I thought it would help prevent NPEs using a >>>>> DefaultConfiguration as the default, but then upon trying that, I found >>>>> myself in an infinite recursion of constructors! >>>>> >>>>> In order to provide a default, we have a few options: >>>>> >>>>> 1. Use NullConfiguration by default. Problem is, this basically means >>>>> "ignore all logging calls". >>>>> >>>>> 2. Lazily create the DefaultConfiguration if configuration is null at >>>>> build() time. >>>>> >>>>> 3. Leak "this" from the DefaultConfiguration constructor into the >>>>> PatternLayout used for the default ConsoleAppender which is used as the >>>>> default appender on the root logger. It's not pure, but sometimes you do >>>>> need to leak an object before it's been fully constructed. >>>>> >>>>> 4. Add another attribute to disable setting a Configuration. I don't >>>>> like this option as it introduces extra complexity and doesn't solve the >>>>> problem of having a null config (and not a NullConfiguration). >>>>> >>>>> Right now, I'm leaning toward 2 and 3 combined. >>>>> >>>>> -- >>>>> Matt Sicker <[email protected]> >>>>> >>>>> >>> >>> >>> -- >>> Matt Sicker <[email protected]> >>> >> >> >> >> -- >> Matt Sicker <[email protected]> >> >> > > > -- > Matt Sicker <[email protected]> > > -- Matt Sicker <[email protected]>
