> 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