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
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
