On Tue, 2005-03-08 at 06:41, Simon Kitching wrote:
> Hi,
> 
> Here are a few comments on logging-1.0.5-alpha1. More to come..

great 

many thanks

> ===========
> 
> Should the WeakHashtable class be rolled into commons-logging.jar?
> It seems easier for users than remembering to deploy the extra jar, and
> should be feasable by having something like this in 
>    Hashtable foo;
>    String version = System.getProperty("java.vm.specification.version");
>     if (versionLessThan(version, "1.3")) {
>       foo = new Hashtable();
>     } else {
>       // use reflection to create instance
>       foo = createWeakHashtable();
>     }
>   
> Or is the reason for having it separate because there is a performance
> hit when using it? If that is so, then file guide.xml should document
> that rather than saying "always deploy it when using java 1.3 or later".

there may be some performance hit but this should only really happen the
first time that a Log is obtained for a new classloader. i doubt that
there will be significant real world performance degradation by using
this jar. be nice to have some figures, though.  

there were two main reasons why it was issued as an optional jar:

1. JCL has always tried hard to be compatible with early JVMs 
2. backwards compatibility is very important
 
the memory problem is only likely to manifest when frequently hot
deploying in certain containers. i'm a little reluctant to use system
property version numbers (since they haven't always been very reliable)
and prefer to give users the explicit choice as to whether they risk
using the new code or not. 

however, maybe it would be a reasonable tradeoff in this case and i
would be willing to be convinced. (i tend to be very conservation when
maintaining components with large installation bases).

opinions anyone?

> ===========
> 
> The current javadoc for the WeakHashtable class doesn't include a
> description of the general problem it's trying to solve (though this is
> well described in the guide.xml).
> 
> I found it rather difficult to understand the description of the
> remaining issue that this class still doesn't handle.

i'm quite fond of the precision of the existing description. however, i
take your point.

> So attached is a proposed patch to the javadoc for the WeakHashtable
> class.

i think i'll probably try to pull something together containing the best
of both. i suppose that the user guide will also need updating...

> BTW, is there a maven command that will actually generate the javadoc
> for the optional classes? 

it should be possible to get the reactor to work but i've never really
had any success using the maven reactor. (probably need to sit down for
half a day or so and really get to grips with it.) if you anyone can
find the cycles, then i'd be happy to see the maven build improved :)

FYI simon: the usually commons process for karma for additional
components for existing committers is just to ask

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to