You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4php-dev@logging.apache.org by Christian Hammers <ch...@lathspell.de> on 2009/08/07 22:31:45 UTC

Thoughts about a comment in LoggerHierarchy...

Hello

Regarding the TODO comment:

    class LoggerHierarchy {
	...
	/* TODO: In log4j is this class not a singleton. Why is it in log4php? */ 
	public static function singleton() {
		static $instance;
		if(!isset($instance)) {
			$instance = new LoggerHierarchy(new LoggerRoot()); 
		} 
		return $instance;
	}

In log4j they use:

 static private RepositorySelector repositorySelector;

 public static LoggerRepository getLoggerRepository() {
    if (repositorySelector == null) {
        repositorySelector = new DefaultRepositorySelector(new NOPLoggerRepository());
        ...
    }
    return repositorySelector.getLoggerRepository();
  }

which looks to me like a Singleton. The repositorySelector is only a container for
the Hierarchy object which is quite the same as our LoggerHierarchy.

bye,

-christian-

Re: Thoughts about a comment in LoggerHierarchy...

Posted by Christian Grobmeier <gr...@gmail.com>.
Hi,

>> I also think, that only one instance is needed of LoggerHierarchy.
>
> "Recent log4j releases support multiple hierarchy trees. This
> enhancement allows each virtual host to possess its own copy of the
> logger hierarchy."
> http://logging.apache.org/log4j/1.2/manual.html - NDC section

I just read this one.

I don't think this requirement is proper for Log4PHP. We don't have
our classes shared within a VM, we have our classes only shared within
a request. However, IF we would need multiple hierarchy trees, then we
would have a problem when using the classic singleton defined with
getInstance in LoggerHierarchy. Cause moving from a single variable in
Logger to an array which holds the hierarchies should not be a big
problem.

Guess this is a point for removing LoggerHierarchy::getInstance.

Christian

Re: Thoughts about a comment in LoggerHierarchy...

Posted by Christian Hammers <ch...@lathspell.de>.
Hello

Apropos:
> I also think, that only one instance is needed of LoggerHierarchy.

I'm too tired to think about this in dept at the moment but coincidently
I just stumbled about the following sentence in the log4j docs:

"Recent log4j releases support multiple hierarchy trees. This
enhancement allows each virtual host to possess its own copy of the
logger hierarchy."
http://logging.apache.org/log4j/1.2/manual.html - NDC section

bye,

-christian-


Am Fri, 7 Aug 2009 22:45:20 +0200
schrieb Christian Grobmeier <gr...@gmail.com>:

> Hi,
> 
> please see my other e-mail with refactoring thoughts before a while.
> Would like to hear your thoughts about that too, since your mail and
> mine is tied together.
> 
> I also think, that only one instance is needed of LoggerHierarchy. But
> its not necessary to have a singleton() method in LoggerHierarchy,
> only the Logger class should care about it. My comment (yeah, I wrote
> it :-)) was referring that the class LoggerHierachy is dealing with
> beeing a singleton itself.
> 
> Looking at Log4Js hierarchy:
> http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Hierarchy.html
> 
> Even they don't have a singleton() method in the hierarchy. Its care
> somewhere else.
> 
> Log4PHP doesn't have a repository selector. This would have been tied
> to the LoggerManager, we removed before a while, cause it has no sense
> (at the moment). Looking at the
> http://logging.apache.org/log4j/1.2/manual.html#defaultInit where
> Log4Js  LogManager refers too, its not mentioned any more. Even the
> fields you can find in the link are marked as deprecated.
> 
> However - to make it easy in PHP world I would suggest to create one
> single instance of LoggerHierarchy in the Logger class and delete the
> singleton method from LoggerHierachy.
> 
> But please see my other mail, where I opened this discussion more
> deeply. I am really interested in what you say.
> 
> Thanks,
> Christian
> 
> 
> 
> On Fri, Aug 7, 2009 at 10:31 PM, Christian Hammers<ch...@lathspell.de>
> wrote:
> > Hello
> >
> > Regarding the TODO comment:
> >
> >    class LoggerHierarchy {
> >        ...
> >        /* TODO: In log4j is this class not a singleton. Why is it
> > in log4php? */ public static function singleton() {
> >                static $instance;
> >                if(!isset($instance)) {
> >                        $instance = new LoggerHierarchy(new
> > LoggerRoot()); }
> >                return $instance;
> >        }
> >
> > In log4j they use:
> >
> >  static private RepositorySelector repositorySelector;
> >
> >  public static LoggerRepository getLoggerRepository() {
> >    if (repositorySelector == null) {
> >        repositorySelector = new DefaultRepositorySelector(new
> > NOPLoggerRepository()); ...
> >    }
> >    return repositorySelector.getLoggerRepository();
> >  }
> >
> > which looks to me like a Singleton. The repositorySelector is only
> > a container for the Hierarchy object which is quite the same as our
> > LoggerHierarchy.
> >
> > bye,
> >
> > -christian-
> >

Re: Thoughts about a comment in LoggerHierarchy...

Posted by Christian Grobmeier <gr...@gmail.com>.
Hi,

please see my other e-mail with refactoring thoughts before a while.
Would like to hear your thoughts about that too, since your mail and
mine is tied together.

I also think, that only one instance is needed of LoggerHierarchy. But
its not necessary to have a singleton() method in LoggerHierarchy,
only the Logger class should care about it. My comment (yeah, I wrote
it :-)) was referring that the class LoggerHierachy is dealing with
beeing a singleton itself.

Looking at Log4Js hierarchy:
http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Hierarchy.html

Even they don't have a singleton() method in the hierarchy. Its care
somewhere else.

Log4PHP doesn't have a repository selector. This would have been tied
to the LoggerManager, we removed before a while, cause it has no sense
(at the moment). Looking at the
http://logging.apache.org/log4j/1.2/manual.html#defaultInit where
Log4Js  LogManager refers too, its not mentioned any more. Even the
fields you can find in the link are marked as deprecated.

However - to make it easy in PHP world I would suggest to create one
single instance of LoggerHierarchy in the Logger class and delete the
singleton method from LoggerHierachy.

But please see my other mail, where I opened this discussion more
deeply. I am really interested in what you say.

Thanks,
Christian



On Fri, Aug 7, 2009 at 10:31 PM, Christian Hammers<ch...@lathspell.de> wrote:
> Hello
>
> Regarding the TODO comment:
>
>    class LoggerHierarchy {
>        ...
>        /* TODO: In log4j is this class not a singleton. Why is it in log4php? */
>        public static function singleton() {
>                static $instance;
>                if(!isset($instance)) {
>                        $instance = new LoggerHierarchy(new LoggerRoot());
>                }
>                return $instance;
>        }
>
> In log4j they use:
>
>  static private RepositorySelector repositorySelector;
>
>  public static LoggerRepository getLoggerRepository() {
>    if (repositorySelector == null) {
>        repositorySelector = new DefaultRepositorySelector(new NOPLoggerRepository());
>        ...
>    }
>    return repositorySelector.getLoggerRepository();
>  }
>
> which looks to me like a Singleton. The repositorySelector is only a container for
> the Hierarchy object which is quite the same as our LoggerHierarchy.
>
> bye,
>
> -christian-
>