avalon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus Crafter <craft...@managesoft.com>
Subject Re: cvs commit: avalon/fortress/container/src/impl/org/apache/avalon/fortress/impl AbstractContainer.java
Date Mon, 30 Jun 2003 13:53:18 GMT
Hi Anton!

On Sun, Jun 29, 2003 at 02:41:41AM +0400, Anton Tagunov wrote:
> cao>                // do the handler lifecycle
> cao>   +            ContainerUtil.enableLogging( targetHandler, getLogger() );
> cao>                ContainerUtil.contextualize( targetHandler, m_context );
> 
> Great you've spotted this! You patch has also made me see that there
> still is more cleanup to do, look:
> 
> public abstract class AbstractComponentHandler
>     extends AbstractLogEnabledInstrumentable ...
> {
>     ...
>     
>     protected LoggerManager m_loggerManager;
>     protected Logger m_logger;
> 
>     public void service( final ServiceManager manager ) ...
>     {
>         m_loggerManager =
>             (LoggerManager) manager.lookup( LoggerManager.ROLE );
>         ...
>     }
>     ...
> 
>     public void initialize() ...
>     {
>         m_logger =
>             m_loggerManager.getLoggerForCategory( categoryName );
>         ...
>     }
>     ...
> }
> 
> AbstractContainer has
> 
>             final DefaultServiceManager serviceManager =
>                     new DefaultServiceManager( getServiceManager() );
>             serviceManager.put( ObjectFactory.ROLE, factory );
>             serviceManager.makeReadOnly();
> 
>             ContainerUtil.service( targetHandler, serviceManager );
> 
> What do we see?
> 
> 1) AbstractComponentHandler both
>    * is AbstractLogEnabledInstrumentable
>    * has it's own m_logger

Yes, just noticed that myself too. :)

> 2) It sort of enables its own logging but as late as
>    in service() method

Yes, but it only enables logging on the internal m_logger reference, the
getLogger() method from AbstractLogEnabledInstrumentable continues to 
return null because from what I can see no call to enableLogging() has been 
made.

> 3) While it expects to find a LoggerManager in service()
>    AbstractContainer fails to push it there

Right - but I think it's in the parent ServiceManager passed into the
locally created one (by getServiceManager()).

> There is also one thing I like about this all: to me it makes
> sense to use a different logging category for the handlers then
> for the container itself.

Yes. I agree.

> I'm at doubt what to do, but clearly some cleanup is still pending :)

Well, one option would be to remove the internal m_logger reference in
AbstractComponentHandler and have initialize call enableLogging() instead.
That would fix the problem I had and would keep the logging objects in sync.

It kind of abuses the lifecycle though - ie. setting the logger object in
initialize() rather than enableLogging(). :(

Another way would be to calculate the handler's logging category in
AbstractContainer.getComponentHandler() and set it that way (ie. relocate
the current AbstractComponentHandler.initialize() logger category
calculation code). That's definitely more 'component oriented' - ie. the
component is passed it's logger, rather than obtaining it itself.

What do you think mate?

Cheers,

Marcus

-- 
        .....
     ,,$$$$$$$$$,      Marcus Crafter
    ;$'      '$$$$:    Computer Systems Engineer
    $:         $$$$:   ManageSoft GmbH
     $       o_)$$$:   82-84 Mainzer Landstrasse
     ;$,    _/\ &&:'   60327 Frankfurt Germany
       '     /( &&&
           \_&&&&'
          &&&&.
    &&&&&&&:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@avalon.apache.org
For additional commands, e-mail: dev-help@avalon.apache.org


Mime
View raw message