commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 38174] - [logging][PATCH] Improvements to wrapper classes
Date Wed, 18 Jan 2006 04:39:04 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38174>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=38174





------- Additional Comments From skitching@apache.org  2006-01-18 05:39 -------
Hi Boris,

Thanks very much for your patch. Unfortunately I'm inclined not to apply this
patch to the next release, for the reasons below. However I agree these points
should definitely be kept in mind when designing JCL 2.0.


The implementation adds overhead to the critical path for logging messages,
including an extra virtual method call and a call to the underlying logger to
determine whether the loglevel is enabled. There's some additional overhead if
the message is actually logged, but that's not so important as the act of really
logging a message is likely to be much larger than the overhead introduced.

The patch is also reasonably large, touching a significant portion of all the
logging classes. That introduces the risk of new bugs; JCL's unit test suite
isn't *too* bad but it's definitely not 100% coverage so we can't just trust the
tests to pick up any issues.

The issue of behaviour when NULL is passed as the object to log is valid; it
would be nice for JCL to impose consistent behaviour rather than leaving it up
to the underlying logging library. However it's not a major issue.

The issue of the message-object to be logged throwing an exception in its
toString is really pretty unusual. Almost all the time, a String or StringBuffer
will be passed here, and they never have problems with toString. In the rare
case where it does occur, it's the caller's fault anyway. Yes, this would be
nice to handle but I don't think the risk of introducing a bug is worth it at
the moment.

Consistency of code - yes that would be nice, but it's not critical.

JavaDoc - again, this would be nice but it's not critical. I would consider a
javadoc-only patch, but can't be bothered to extract the javadoc bits out of the
attached all-in-one patchfile.

Centralising code for future maintainability - well, yes good idea. But this
code doesn't have a lot of lifetime left; hopefully the release after next will
be JCL 2.0, with significant work done on all parts of the library.


Anyone else got an opinion on this?

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


Mime
View raw message