--- Simon Kitching <[EMAIL PROTECTED]> wrote:
> On Sun, 2005-05-22 at 21:01 +0100, robert burrell
> donkin wrote:
> > On Sun, 2005-05-22 at 10:43 +0000,
> [EMAIL PROTECTED] wrote:
> >
> > <snip>
> >
> > > protected boolean
> isJdk13LumberjackAvailable() {
> > >
> > > + // note: the algorithm here is
> different from isLog4JAvailable.
> > > + // I think isLog4JAvailable is
> correct....see bugzilla#31597
> >
> > +1
> >
> > could do with going through all the isAvailable's
> and checking whether
> > their algorithms are correct. however, i suspect
> that brian's approach
> > will be needed to deal correctly with some
> circumstances. if no one
> > feels like volunteering should probably record
> this in bugzilla so we
> > don't lose track...
>
> If by "brian's approach" you mean creating an
> instance of the logger
> class in order to test whether that logging lib is
> *really* available, I
> agree.
>
> Not only is it more reliable, but it's a cleaner
> solution; currently the
> LogFactoryImpl class is making *assumptions* about
> what classes the
> various logging adapters depend on. That information
> should be only in
> the logging adapter class.
>
> The only problem with creating an instance of the
> logger is that we
> would have to pass a category string to the logger
> constructor, and
> therefore must build an assumption into
> LogFactoryImpl about what
> category names are valid for the underlying logger.
> Can we assume that
> an empty string is a valid category for all logger
> libraries? Can we
> assume that "apache" or "org.apache.commons.logging"
> are valid category
> strings? Perhaps some loggers only accept valid URLs
> as
> categories...yes, I'm playing devil's advocate a bit
> here.
It's a good point; the "assumed" category names in the
various isXYZAvailable method is my biggest dislike
about the patch I submitted. Actually, if an approach
like the patch were adopted, I think we should
consider marking those methods as deprecated (they're
not used in the patched LogFactoryImpl itself) and
dropping them at the next release with a point scope
that allows it.
In general, if any significant work on LogFactoryImpl
is done I think a review of its API would be good.
This seems to me to be the kind of class that if
someone wants a different implementation, they should
write a different implementation and not count on a
lot of behaviors exposed by a superclass.
Of course, any kind of deprecation doesn't solve the
immediate problem.
Looking into it more, I remembered that
LogFactoryImpl.getInstance(Class clazz) just calls
getInstance(clazz.getName()) and thus already creates
a requirement that Log implementations support fully
qualified class names. So any adapter that doesn't do
this must be designed to work with a custom
LogFactory. So, the potential list of users affected
by this issue is smaller -- only those with custom
LogFactory implementations and Log implementations
that don't support class names.
> I guess we
> could always say that the writer of the logging
> adapter is required to
> return a valid logger instance for category "", even
> if that is not
> normally a category that is valid to the underlying
> library.
>
Half-baked idea to throw out before I run. Add to
LogFactoryImpl:
/**
* Returns a category that can be used to test whether
* an instance of a <code>Log</code> can be created.
* <p>
* This default implementation returns the fully
* qualified name of this class. Subclasses should
* override this if they support <code>Log</code>
* implementations that cannot handle this category.
* </p>
*/
protected String getTestCategory() {
return getClass().getName();
}
All the various isXYZAvailable methods would call this
method when creating a test instance.
Brian
> Regards,
>
> Simon
>
>
>
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> [EMAIL PROTECTED]
> For additional commands, e-mail:
> [EMAIL PROTECTED]
>
>
__________________________________
Yahoo! Mail Mobile
Take Yahoo! Mail with you! Check email on your mobile phone.
http://mobile.yahoo.com/learn/mail
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
