commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Remy Maucherat" <r...@apache.org>
Subject Re: Problems with commons-logging
Date Sat, 02 Feb 2002 19:42:23 GMT
> Here's a list of problems I see in the current logging API.
>
> Given that a lot of packages will eventually depend and use this API,
> I believe at least the first issues should be resolved before a release.
>
> - security: getLogNames() and getInstance() are evil and unacceptable.
> Both log4j and logkit have solutions that allow safe use in a container
> environment, i.e. support of isolation for the users of the API
> ( one app using the package can't mess with another app's logging ).
> I'm -1 on releasing with this whole in it.

You can always hide it in the core of the container. And the SM will protect
you :)
I agree it may be a problem in some cases, but it won't be for Tomcat.

I don't see a problem with getInstance, though. What's the problem ?
Using getInstance, or just constructing the object with a new seems
equivalent to me.

> - static methods in LogSource. I suppose LogSource is a sort of
> factory - the pattern used here is completely unnatural ( or at least
> different from most APIs in use ).

Static everywhere, actually ;-)
There's some syncing needed in there, also (the loggers are in a HashMap).

> - I would prefer Log to be an abstract class or even to be a
> normal class, with the minimal logger - and have LogSource return
> a particular impl. If static methods are used, it's cleaner to put
> them in Log, and let the LogSource ( I would rename it LogManager )
> be used behind the scene. I.e.
>      Log log=Log.getLog( name );
> and getLog() will find a LogManager, etc.
>
> - It's missing a log() method that takes a level parameter.
> Having 5 fixed levels is fine for most apps, but not extensible enough.
> Most loggers provide such a thing.

That sounds like a good idea.

> - also in the 'container use' case, given that the Logger will
> probably be used by many components it's likely it'll end up at top
> level loader. It would be important for different apps to use different
> logger adapters if they want to - the current solution allow only
> one.

Yes, that could be useful for more flexibility.

> - Given that it is a facade, it would need some way to pass at least
> config info to the underlying logger ( at least setProperty like ).
> Some logger may not need any, but if they do it'll require using
> internal APIs.

I don't see very well how to configure the logger either. In Catalina and
Slide, we just instantiate the logger directly, so there's no such problem.

> - I don't like the idea of constructors with a  param. All other APIs
> use a no-param constructor. You can easily call a setter if you need to.

Vey good comment also.

> - pluggable mechanism for finding the impl. Right now everything is
> hardcoded. Reading a properties from CLASSPATH or a similar mechanism
> is needed. ( jaxp style preferable - i.e. java services manifest )

That can be added later IMO.

> - Separation of interface and impl - the 2 classes that 'matter'
> are Log and LogSource, everything else should be in a different package.
> It'll get messy long term, and it's harder to read.

That could be a good idea also (stop having good ideas, you create too much
trouble ;-)).

> - I would prefer for the base impl to be JDK1.1 compatible. There is
> no valid reason to exclude JDK1.1 usage - Hashtable can be used
> without any problem, there is nothing performance critical here.

+1 for Hashtable, since it would also fix the possible syncing issues.

> - makeNewLogInstance comment and impl are completely of sync.
>
> - no support for i18n-style messages. Probably not a big deal
> for the first release, but it would be nice to think about it ( I know
> log4j can do that, I assume other as well ).

You mean have the logger handle the message bundle for you, and just give it
the message key (+ params) ?

Remy


--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message