Hi Mandy, On 14/02/19 20:20, Mandy Chung wrote:
Fixing the implementation of Handler::isLoggable to return false if null to match the specification seems similar risk to changing the spec. What do you think?
How so? I mean - if no line of code is changed, then surely we can't break existing code? But on second thought I believe I may have made the wrong choice her. See below...
In addition, logging tends to be gracious as it's a cross-cutting concern and not to disturb the application runtime until it's a runtime issue. So I think isLoggable accepting null may be by design.
I agree that returning false would be a much better implementation than throwing NPE. StreamHandler already does that. The only concrete implementation (in the JDK) of Handler that doesn't return false is the memory handler.
Handler.publish(LogRecord) specifies to ignore null record and the implementation calls Handler.isLoggable(record). If the spec is changed to throw NPE, Handler::publish will need to change too. This impacts all APIs that accept null LogRecord while assuming isLoggable(null) returns false.
That is a good point.
I think normal logging purpose would pass non-null records otherwise this issue should have been reported long ago.
Well... That and the fact that FileHandler, ConsoleHandler, and friends are all StreamHandler (and return false). Maybe you're right - and changing the implementation is the right thing to do. Let me redo my fix and see if the JCK complains. Thanks for the feedback! best regards, -- daniel
