search

apache / logging-log4j-kotlin

A Kotlin-friendly interface to log against the Log4j API
https://logging.apache.org/log4j/kotlin
Apache License 2.0
45 stars 11 forks source link

LoggingFactory: Make logger() used cached loggers by default #20

Closed sschuberth closed 1 year ago

rocketraman commented 2 years ago

Did you notice a problem that this was intended to solve?

sschuberth commented 2 years ago

Did you notice a problem that this was intended to solve?

No, it just felt more natural to me to use the cached loggers by default as a potential performance improvement.

sschuberth commented 1 year ago

Actually, it probably would make even more sense to turn the extention function into an extension property instead. Something like

val <reified T : Any> T.logger: KotlinLogger
    inline get() = cachedLoggerOf(T::class.java)

What do you think @rocketraman?

jvz commented 1 year ago

I kind of like that idea. I'm not super familiar with API compatibility with Kotlin (besides looking up how default argument values are translated to Java; turns out that adding more named parameters is indeed an ABI change as you might expect despite the API staying source-compatible).

jvz commented 1 year ago

Ok, now that I've loaded the code back up again (and after using Kotlin myself a lot more lately), I see where you're going with this. If we leave the function in place as always constructing a KotlinLogger, then a property can use a cached logger which seems intuitive. For example:

inline fun <reified T> T.logger(): KotlinLogger = loggerOf(T::class.java)

inline val <reified T> T.logger: KotlinLogger
  get() = cachedLoggerOf(T::class.java)

The docs for the T::logger function explicitly states that it's for logger instantiation. Adding a property for the cached logger seems like an interesting API alternative to extending the Logging mixin. Kotlin is all about flexibility here, so let's go for it!

jvz commented 1 year ago

I've added the extension property in #29.