Misleading statements in Introduction to log4cxx

• Key: LOGCXX-16
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Curt Arnold
• Reporter: Curt Arnold
• Component/s: Documentation
• Affects Version/s: 0.9.7
• Fix Version/s: 0.10.0

Description

http://nagoya.apache.org/eyebrowse/ReadMsg?listName=[e-mail]&msgNo=196

From: Curt Arnold <[e-mail]>
Subject: Misleading statements in Short Introduction to log4cxx
Date: Tue, 6 Jul 2004 00:06:51 -0500


In http://logging.apache.org/log4cxx/manual/Introduction.html, there
are several statements that may mislead developers just starting with
log4cxx (like me), some are obviously hold overs from the short
introduction to log4j. I scanned the entire archive of the
log4cxx-user and log4cxx-dev mailing lists and didn't see any
discussion on these issues.

Logger prototype:

The logger prototype shows debug, info, warn. error and fatal as having
a single string reference argument when in reality they take three
arguments, a string reference, a const char* for the source file and a
int for line number. The last two have default values, so it is easy
to not realize that if you use log4j style log statements, you do not
have source files and line numbers available, but you do if you use the
LOG4CXX_level macros.


Performance:

> 1. Logging performance when logging is turned off.
>
> When logging is turned off entirely or just for a set of levels, the
> cost of a log request consists of a method invocation plus an integer
> comparison. On a 233 MHz Pentium II machine this cost is typically in
> the 5 to 50 nanosecond range.
>
> However, The method invocation involves the "hidden" cost of
> parameter construction.

The example goes on to show a case where these is a string
concatenation. However, since log methods use String references, there
will likely be a string destruction and destruction in a simpler case
such as:

logger->info("Hello, World");

I believe the cost estimate is just a hold over from the log4j
document, even if it is not, a 233 MHz Pentium II is not a typical
target platform.


> or in a simpler way:
> LOG4CXX_DEBUG (logger, "The user named [" + strName + "] is logged" );

 From the context, a developer is likely to assume (incorrectly) that
the definition for LOG4CXX_DEBUG is something like:

#define LOG4CXX_DEBUG(logger, msg) if (logger->isDebugEnabled()) {
logger->forcedLog(log4cxx::Level::DEBUG, msg); }

When it is actually:

#define LOG4CXX_DEBUG(logger, message) { \
        if (logger->isDebugEnabled()) {\
        ::log4cxx::StringBuffer oss; \
        oss << message; \
        logger->forcedLog(::log4cxx::Level::DEBUG, oss.str(), __FILE__,
__LINE__); }}


I'd be interested in the rational behind the use of the StringBuffer in
this macro instead of relying on the implied String constructor. I
think this would allow you to do unexpected things like log arrays or
other complex types by having a prototype like:

const StringBuffer& operator<<(StringBuffer& lhs, SomeOtherType&rhs);

available and then do:

SomeOtherType foo;

LOG4CXX_DEBUG(logger, foo);


However, I'm not sure this is a great idea and there is nothing to hint
at it in the short introduction.

Suggestions:

Obsolete performance data should be removed from the Short introduction
and either replaced with relevant performance data or the benchmark
page should be referenced?

Since logger->debug(msg) and LOG4CXX_DEBUG(logger, msg) have some
pretty significant differences, either they should be made equivalent
or the differences should be discussed and recommendations given.

If LOG4CXX_DEBUG is preferred over logger->debug(), then the examples
should be rewritten.

I'd consider removing the default values for the file and line
parameters on the Logger.debug and similar methods.

Comments

[Curt Arnold]

Committed changes to docs/introduction.dox and made a stab at updating the web site (by committing the modifications to xdocs/manual/introduction.html) and have an unjustified hope that the web site will be refreshed eventually.