> On Nov 23, 2023, at 18:31, Piotr P. Karwasz <piotr.karw...@gmail.com> wrote:
> 
> Hi Matt,
> 
>> On Tue, 21 Nov 2023 at 23:22, Matt Sicker <m...@musigma.org> wrote:
>> 
>> This sounds like it might be a good basis for figuring out a parallel v3 API 
>> for a “hard to mis-use” style API. However, once you go that route, you 
>> start to wonder how useful templated log messages are when you can capture a 
>> lambda instead. Parameterized log messages might work better as structured 
>> log messages, something that is awkward to use in the API at the moment.
> 
> If we'll create a separate `v3.Logger` interface I would clean it up
> from many methods, e.g.:
> 
> * getLevel() and getName(): how are these useful for the user? An `if
> (logger.getLevel() == Level.INFO)` should be replaced by `isEnabled`,

Sorry but I disagree. 
The getLevel method is useful when the level to compare to is not known in 
advance. 

> * getMessageFactory() and getFlowMessageFactory() (the latter is my
> fault): again these are not useful to the user. If I need a message
> factory, it will be a different message factory,

Agreed.

> * printf(): a better approach is to use StringFormatterMessageFactory
> for the logger,

I see your point, but in my experience the printf method is rarely used, only 
for specific use cases; most logging in the same class would use the info(), 
debug() etc methods. 
Without printf users would need to declare a separate logger. 
The printf method is a nice convenience. 


> * catching(Throwable): can be replaced with `error(Object)` and the
> semantics described in this thread,

Agreed.

> * throwing: does anybody use it? Maybe it could stay,

I never use it. 

> * entry/exit, traceEntry/traceExit: I can not imagine using these on
> each method (or important method). I'd rather use AspectJ pointcuts
> instead (or a @LogTrace annotation),

Agreed. 

> * methods that use `MessageSupplier` like `info(MessageSupplier)`:
> couldn't these be integrated into the logic of `info(Supplier)`?

Yes. 

> * the `is*Enabled` methods are prone to misuse: a snippet like:
> 
> if (logger.isDebugEnabled()) {
>  logger.debug(MARKER, "Hello world!");
> }
> 
> will not print any message if the level of the logger is less specific
> than DEBUG, even if the user asks for **all** MARKER messages to be
> printed.

Interesting. 
By the way; isn’t that behavior the same even if there’s no “if” condition 
around the logger.debug(MARKER, "Hello world!");
statement?


> 
> IMHO opinion `v3.AbstractLogger` should only have 2 abstract methods:
> * logMessage(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
> * isEnabled(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
> 
> Piotr

Reply via email to