Comments inline.
On Tuesday 19 November 2002 08:23 pm, Kevin Smith wrote:
> Yes. That's an option, too. But this all started off as me volunteering to
> change strings that had been hard coded in LogFactory.getLogger() into
Shoudl read: "change strings that had been hard coded in
LogFactory.getLogger() calls into"
> something a bit more robust without doing a full-blown refactoring.
>
> Are there any plans to run xindice inside of Phoenix? That might make the
> refactoring effort worth it.
>
> --Kevin
>
> On Wednesday 20 November 2002 01:18 am, Gary Shea wrote:
> > I'm jumping into the middle of a conversation that I haven't followed
> > closely, so this could be inapplicable, but it seems to me that there is
> > an option #4, namely Avalon-style explicit provisioning of a Logger when
> > the DatabaseManager is constructed.
> >
> > Just a thought...
> >
> > Gary
> >
> > On Tue, 19 Nov 2002, at 20:00 [-0000], Kevin Smith
>
> ([EMAIL PROTECTED]:
> > > Here's the threading issue I see:
> > >
> > > public class DatabaseManager
> > > {
> > > private transient Logger logger;
> > >
> > > public Database getDatabase() {
> > > Logger logger = getLogger();
> > > logger.debug("Getting database");
> > > // do something here and return database
> > > }
> > > protected Logger getLogger() {
> > > if(logger == null)
> > > logger = LogFactory.getLogger(getClass());
> > >
> > > }
> > > }
> > >
> > > If two threads call getDatabase() at the same time, there is a chance
> > > that you could wind up allocating two loggers when only one is needed.
> > > Since the logger is thread-safe, we need to make getting a reference to
> > > a logger thread-safe, too.
> > >
> > > There are three ways of doing it:
> > >
> > > 1) Synchronize getLogger() - This introduces synchronization overhead
> > > every time you need a logger.
> > >
> > > 2) Synchronize all methods that call getLogger() - Ugly.
> > >
> > > 3) Make the logger a static variable - This moves the initialization of
> > > the logger to when the class is loaded and thus, removes any threading
> > > issues. The penalty for such a refactoring is having one line of
> > > duplicate code in each class:
> > >
> > > private static Logger logger =
> > > LogFactory.getLogger(Database.class.getName());
> > >
> > > That's why I was suggesting using #3.
> > >
> > > --Kevin
> > >
> > > On Tuesday 19 November 2002 07:52 pm, Kevin Ross wrote:
> > > > >Obtaining loggers might be a bad example. All I'm saying that in my
> > > >
> > > > experience, this kind of resource initialization scheme is better off
> > > > being thread safe from >the start. It eliminates a whole family of
> > > > bugs that you need to worry about down the road.
> > > >
> > > > Agreed, but in the case of logging, I believe you should err on the
> > > > side of performance for the greater good, the 80%+ case. Bothering
> > > > with paranoid initialization (synchronization) makes everyone
> > > > instance pay the price.
> > > >
> > > > Just got your next message. I'll say it one more time, WHAT thread
> > > > safety issues???? The logger is responsible for thread safety. I
> > > > don't implement synchronization when initializing or using a Logger,
> > > > because it is unnecessary for the client to do so. I haven't for
> > > > years now, and I haven't had one problem in organizations with
> > > > traffic from a few thousand hits a day, to a few million.
> > > >
> > > > Duplication of code is one of the fundamentally wrong things to do in
> > > > OO programming, and I agree, therefore I stay away from it.
> > > >
> > > > -Kevin Ross
> > > >
> > > > Kevin A. Smith wrote:
> > > > > I just thought I'd bring it up. Personally, I err on the side of
> > > > > paranoia when I can. I don't enjoy debugging threading issues.
> > > > >
> > > > > If the LogFactory is recycling logger instances, then at most you
> > > > > pay the price of one extra method call. BUT, if the LogFactory is
> > > > > creating a new logger per call, then you're (potentially) paying
> > > > > the price of creating an object that won't be used and an extra
> > > > > method call.
> > > > >
> > > > > Obtaining loggers might be a bad example. All I'm saying that in my
> > > > > experience, this kind of resource initialization scheme is better
> > > > > off being thread safe from the start. It eliminates a whole family
> > > > > of bugs that you need to worry about down the road.
> > > > >
> > > > > --Kevin
> > > > >
> > > > > -----Original Message-----
> > > > > From: Kevin Ross [mailto:[EMAIL PROTECTED]
> > > > > Sent: Tuesday, November 19, 2002 2:30 PM
> > > > > To: [EMAIL PROTECTED]
> > > > > Subject: Re: LogFactory.getLogger()
> > > > >
> > > > > Threading issues?? I knew someone would bring that up. Thread
> > > > > safetyness is the responsibility, gladly accepted by a logger
> > > > > instance, not on the classes using it. There is NO reason why
> > > > > we need paranoid checking here to only get one instance, since it
> > > > > is the responsibility of the logging implementation to even provide
> > > > > the instance in the first place. Do you really think you'll get
> > > > > different instances? Probably not, worst case scenario, you call
> > > > > Logger.getLogger() twice and initialize your instance twice in a
> > > > > race condition. Since it is of zero consequence either way, there
> > > > > is no reason to do it differently. If you are particularly
> > > > > worried, just do:
> > > > >
> > > > > private transient Logger logger =
> > > > > LogFactory.getLogger(getClass());
> > > > >
> > > > > You CANNOT use MyClass.class and have MySubClass.class show up
> > > > > in the log statements, since this is static. You must use dynamic
> > > > > initialization.
> > > > >
> > > > > -Kevin
> > > > >
> > > > > Kevin A. Smith wrote:
> > > > >> That doesn't look thread-safe to me. You'd need to somehow
> > > > >> synchronize access to getLogger() to eliminate a
> > > > >> race-condition on the if(logger == null) test.
> > > > >>
> > > > >> While its a bit more typing, I prefer:
> > > > >>
> > > > >> private static Logger logger =
> > > > >> LogFactory.getLogger(MyClass.class);
> > > > >>
> > > > >> or
> > > > >>
> > > > >> private static Logger logger =
> > > > >> LogFactory.getLogger(MyClass.class.getName());
> > > > >>
> > > > >> No threading issues and is pretty easy to read and understand.
> > > > >>
> > > > >> --Kevin
> > > > >>
> > > > >> -----Original Message-----
> > > > >> From: Kevin Ross [mailto:[EMAIL PROTECTED]
> > > > >> Sent: Tuesday, November 19, 2002 2:18 PM
> > > > >> To: Vladimir R. Bossicard; [EMAIL PROTECTED]
> > > > >> Subject: Re: LogFactory.getLogger()
> > > > >>
> > > > >> As I mentioned in my previous message, this is a poor way
> > > > >> to obtain a logger, especially if you have deep inheritance
> > > > >> heirarchies. Since I'm a big fan of OO programming, and my
> > > > >> heirarchies are especially deep, I find the static way of
> > > > >> obtaining a logger invalid.
> > > > >>
> > > > >> Try this:
> > > > >>
> > > > >> public class AbstractLoggable{
> > > > >>
> > > > >> private static transient Logger logger;
> > > > >>
> > > > >> protected Logger getLogger(){
> > > > >>
> > > > >> if( logger == null ){
> > > > >>
> > > > >> logger = Logger.getLogger(getClass());
> > > > >> }
> > > > >>
> > > > >> return logger;
> > > > >> }
> > > > >> }
> > > > >>
> > > > >> whallah! efficient OO programming with an accessor for
> > > > >> subclasses and the proper class name to go with it!
> > > > >>
> > > > >> -Kevin Ross
> > > > >>
> > > > >> Vladimir R. Bossicard wrote:
> > > > >>>>Logger.getLogger(getClass()) at least, whether or not you use the
> > > > >>>> string name is of no consequence.
> > > > >>>
> > > > >>>have you ever tried to call a non-static method within a static
> > > > >>> reference?
> > > > >>>
> > > > >>>if you get this working:
> > > > >>>
> > > > >>> private static Log log = LogFactory.getLog(getClass());
> > > > >>>
> > > > >>>I give you a high-five.
> > > > >>>
> > > > >>>-Vladimir
> > > > >>>
> > > > >>>--
> > > > >>>Vladimir R. Bossicard
> > > > >>>www.bossicard.com
