On Fri, 2004-12-10 at 10:29, David Graham wrote:
> "Enterprise" is marketing speak for "expensive".  It has no technical
> meaning so including that word in class names is rather confusing.  It
> looks like the new functionality is related to i18n of messages so naming
> the classes something like I18NLog or GlobalizedLog would be more
> appropriate.
> 

+1 from me. Enterprise seems rather too grand a word for the fairly
simple proposed changes.

Actually, I would suggest a classname of "Logger" rather than Log. This
brings it even more in line with JSR-47.

And I would suggest making the new interface (currently "EnterpriseLog")
an abstract class rather than an interface. This means that in future
new functionality *can* be added without introducing a new interface +
associated factory. Several commons projects are moving away from
interfaces and towards abstract classes for this reason. Of course this
must be used appropriately, but I can't see anyone wanting to subclass
from both EnterpriseLog *and* some other base class.


Regarding the "Globalisation" changes, I'm generally in favour of the
proposal. In particular, using MessageFormat-style strings eg "a {0}
problem occurred" allows the underlying implementation to perform the
string formatting only after checking whether the loglevel is enabled,
thus allowing some of the
  if (log.isDebugEnabled()) {
    log.debug("a " + description + " problem occurred.");
  }
type code to be simplified. I am a little concerned about how the
resource-bundle gets selected, though. As mentioned later,
commons-logging currently needs *no* configuration, and I think that is
a big feature.

Regarding the addition of "entry/exit" logging APIs, I'm also in favour.
The code seems trivial, and it can be mapped to "TRACE" level for
logging implementations that don't provide "FINER" equivalents. It also
seems to me that:
  log.enter("MyClass", "MyMethod", "Entering method");
is nice and readable.

== discovery

Regarding changes to the commons-log "discovery" process, I'm far less
convinced. I personally like the fact that commons-logging works fine
without any commons-logging-specific configuration. Essentially, a
person deploying an application built with commons-logging doesn't need
to know that commons-logging is used by the application. They just
configure whatever logging library is available in the environment the
application is being deployed into, and commons-logging auto-detects
that library and uses it.

Having an "override" option so that commons-logging can be *forced* to
use a specific logging implementation is not a bad idea - but there is
such a feature already. Users can set system property
org.apache.commons.logging.LogFactory, or use standard Service
Discovery, or place a commons-logging.properties file in the classpath,
as described in the javadocs for LogFactory.getFactory(). 

Can you explain what functionality you need for controlling "discovery"
that is not provided by the above?

== Class Loaders

I haven't read this thoroughly yet, but I see you talk about walking
classpaths to find config files. I presume you are *not* talking about
configuration files to be processed by the underlying log
implementation? Commons-logging has *never* been involved in configuring
concrete log implementations, merely about determining which concrete
logging implementation to delegate logged messages to. Ok, it does have
configuration for the SimpleLog logger, but that is a special case.

== Repackaging

Separating out the various adapters to the concrete logging
implementations has been proposed before. I'm not wildly keen on the
idea myself, as it break the principle that deployers of an application
using commons-logging don't need to care that the app uses
commons-logging, as long as the concrete log implementation available
has an adapter already bundled with commons-logging.

== Misc

Being able to query the logger implementation: you can do this already
via "log.getClass().getName()" -->
"org.apache.commons.logging.impl.Log4jLog" or whatever. But if you can
provide a use-case where a nicer string would be useful, then I can't
seen any major problem with a getLogImplName() method on the new
EnterpriseLog/Logger/whatever interface.

By "an Assert", do you mean:
  log.assert(1+1==2, "Mathematics has failed");
If so, I'm neutral on this. Clearly, after logging the message, some
kind of error would need to be thrown. It can't be
java.lang.AssertionError, because that is only available in java 1.4+.
And any other type would bind the application to commons-logging at the
point it tried to handle the error, even if that class wasn't otherwise
using commons-logging.

I don't understand your point re "naming" loggers. Can you clarify?

Regards,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to