At 07:08 AM 12/1/2004, Jacob Kjome wrote:
Hi Ceki,

Unfortunately, I think you are still missing my point. The changes to the RepositorySelector interface are good. The change to the LogManager is a bit mysterious. I guess it is trying to protect against overriding the default logger repository of custom selectors which initiate a default repository in their constructor?

Your conclusion is correct as long as it is limited to the scope of LogManager's static initializer.


Because that's the only way the default repository would actually exist in selectors instantiated within the static initialization code of LogManager.

Absolutely.

I guess that might be a legitimate check? Can't hurt either way. However, it doesn't address my original point at all, though I can use the same code change from the static initialization code within the LogManager.setRepositorySelector() for the same effect there, which is what I was trying to point out in the first place. To make it abundantly clear what I am talking about, look at the change I just committed. I'm positive this is correct and it fixes all the null pointers which, BTW, cause all apps in Tomcat to cease to work. It is clearly the right thing to do. If you really disagree, you can always back it out, but I can't see why you would want to.

It fixes the null pointer problem, for sure. However, introducing side effects like you have done eventually become part of the expected behavior of your API. You can't change it without breaking code downstream. Forgetting to set the default repository will result in a fairly immediate crash of the application. So anyone travelling on the clue train will figure our that you have to set the default repository in a custom selector. It's not rocket science.


Bottom line is that external initializing code should never ever be able to bring down every app in the server by simply forgetting to set a default logger repository on the custom selector. We shouldn't override a default logger repository that has been set, but we should definitely make sure it doesn't stay null if it wasn't set.

It's not our problem to fix. Unmaintainable code is born trying to compensate for other people's mistakes. Keeping the hocus-focus level low eventually makes a big difference with regards to code maintainability.


And the default repository of choice is the same one that was on the previous selector. That way, the transition is seamless. Anyway, take a look, and try this under Tomcat-5.5.4, not Tomcat-5.0.xx. Also, the problem would never show itself if you started up by passing the selector name in via the -D parameter on the command line. It would only occur in the case where the default selector was being used and then some initializer code called LogManager.setRepositorySelector().

That is correct but probably not relevant. I am repeating myself but the RepositorySelector API is designed to be used by developers of Application Servers or 1st class passengers on the clue train. They are expected to look at the source code or read the documentation.



Jake

-- Ceki G�lc�

The complete log4j manual: http://qos.ch/eclm
Professional log4j support: http://qos.ch/log4jSupport




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



Reply via email to