Paulo, I've seen you mention a couple of times that you consider singletons dangerous. Would you care to elaborate? Is it because you're concerned that people can't write thread-safe code correctly? Or because correct thread-safe code affects concurrency? Or something else?
Thanks, Donnie > -----Original Message----- > From: Paulo Gaspar [mailto:[EMAIL PROTECTED]] > Sent: Sunday, February 03, 2002 8:19 PM > To: Jakarta Commons Developers List; [EMAIL PROTECTED] > Subject: RE: Problems with commons-logging > > > Answer inline: > > > -----Original Message----- > > From: Scott Sanders [mailto:[EMAIL PROTECTED]] > > Sent: Sunday, February 03, 2002 2:18 AM > > > > On Sat, Feb 02, 2002 at 08:33:46AM -0800, [EMAIL PROTECTED] wrote: > > > > > - 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. > > > > > > > I can see how getLogNames() could be considered evil, and I do > > not see what it gains us. I am +1 on remving getLogNames(). > > +1 > > > Are you saying that with getInstance(), we should remove it and > > just use newLogInstance()? I am also fine with this, albeit a +0. > > I am still not sure I understand this one. > (So, I probably don't.) > > > > > - 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 ). > > > > Why don't we rename LogSource to LogFactory, and change > > makeNewLogInstance() to newInstance()? +1 on this as well. > > I believe this would be consistent with things like JAXP, correct? > > +1 > > Also, as I mentioned 2 posts ago on this thread, any Singleton should > be avoided. (Some static methods might make sense but Singletons are > usually dangerous.) > > > > > - 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. > > > > > > > I don't see the problem with having a factory take care of this, > > so I am -0. I like that Log is an interface. > > I am -1. I really like it being an interface. > > > > - 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. > > BUT I do not see how to port the extensibility across APIs. For the > time being I am -1 on extensibility and +1 on having level constants. > > > I will code this up. Thanks for the comment. +1. I am assuming > > void log(int level, Object message) and the proper Throwable > > sister method?. > > Scott, I have this coded but no CPU time (mind frame) to adapt it. > Would it be ok if I send you my code and you take a look at it? > > > > > - 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. > > > > > > > Why would this end up in the top level loader? I do not > > understand this. Could you explain more please? > > > > > - 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 am -1 on walking the config line. No config. None. This API > > intends to mask all of this and allow a component to just log. > > The container using the component will be required to configure > > logging. We are not trying to replace LogKit/Log4J, we are only > > trying to replace System.out.println(), IMHO. Besides, that is > > orhtogonal IMHO, and if logging does this, it can do this in a > > later release, on a separate interface. > > I +1 on config, although on a separate/separable package. Again, > take a look on my previous post on this thread for details (not > my last one but the previous). > > > > > - 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. > > > > Fine. Done. +1 > > > > > - 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 ) > > > > > > > It IS pluggable. Just set the org.apache.commons.logging.log > > property with your Log impl. Could you code up the jar services > > manifest code to augment this? Thanks. > > I also rumble about this one on that previous post a mentioned above. > > > > - 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. > > > > -0. I don't really see the need to break this up. I agree that > > only 2 classes really 'matter', but I do not see that as a reason > > to move them. If someone else wants to, I wont stop them. Are > > you suggesting something like an o.a.c.l.impl package? > > +1 > > I would call it o.a.c.l.wrappers since other implementation bits > might be added (like that o.a.c.l.config I am defending). > > > > > - 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. > > > > Consider this done. > > +1 > > > > - makeNewLogInstance comment and impl are completely of sync. > > > > I will look into this. > > > > > - 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 ). > > > > > > > -0. I personally have no intention of supporting this. I think > > that commons-logging is just a simple conduit to a logging > > implementation, i18n is the concern of both sides, but not the > > middle. If we keep the Object as the log parameter, I don't see > > how we are 'missing out'. > > -1 > > The official (SUN) i18n APIs are insufficient for many apps and > that is why alternative i18n APIs are popping up like mushrooms > all around Jakarta. It would not be nice to favor any of them. > > IMO, a i18n logger could be the subject of another component (or > several - one per i18n API) based on this one. > > I also mentioned this one on my previous post. > > I have an AbstractLoggable class that supports i18n but I will > NOT contribute it to this package. > > ... > > > Have fun, > Paulo Gaspar > > http://www.krankikom.de > http://www.ruhronline.de > > > > -- > To unsubscribe, e-mail: > <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> > > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
