-
-
Notifications
You must be signed in to change notification settings - Fork 124
Description
Note: I know that's an old discussion, but just to revise it again.
Problem
Currently, the "idiomatic" way to create logger is a top-level variable.
import io.github.oshai.kotlinlogging.KotlinLogging
// Place definition above class declaration, below imports,
// to make field static and accessible only within the file
private val logger = KotlinLogging.logger {}
- Lot's of JVM backend teams prefer to have private variables inside the class. What if we have couple of private static variables, should we also place them as a top level declaration? And what if we have instance private variables as well? This introduce inconsistency and hard-to-read code base, since some of the variables could be as a top-level, some of them as a companion object and some of them as a instance variables.
import io.github.oshai.kotlinlogging.KotlinLogging
private val logger = KotlinLogging.logger {}
class MyClass {
private companion object {
private val someStaticVar = 1
private val anotherStaticVar = 2
}
private val someVar = 3
}
- Let's add to the example above a combination with long doc on top of the class that's move out completely the logger from view.
import io.github.oshai.kotlinlogging.KotlinLogging
private val logger = KotlinLogging.logger {}
/**
* very long Doc
*/
class MyClass {
private companion object {
private val someStaticVar = 1
private val anotherStaticVar = 2
}
private val someVar = 3
}
- Exposes the top-level class to the public API.
If I'm writing a Java library, adding a variable at the top level introduces an public empty top-level class. In recent version of Kotlin we can "embed" the top-level variable using@file:JvmName
@file:JvmName("MyPublicAPI") // may work in recent kotlin versions
package example
import io.github.oshai.kotlinlogging.KotlinLogging
private val logger = KotlinLogging.logger {}
class MyPublicAPI {
}
-
Top level variables introduce additional top level class (if we are not using
@file:JvmName
the same as class name). It just makes the final jar bigger. -
Before Kotlin 2.0 each lambda functions compiled to the anonymous classes, that also makes final jar bigger, because for every logger we need a lambda. https://kotlinlang.org/docs/whatsnew20.html#generation-of-lambda-functions-using-invokedynamic
-
There is ongoing proposal about static namespace/static variables and current syntax does not allow to migrate (with IDEA/compiler hint) from this syntax:
class MyClass {
private companion object {
private val logger = KotlinLogging.logger(this)
}
}
to this
class MyClass {
private static val logger = KotlinLogging.logger(this::class) // also can be optimized with "inline .class syntax"
}
- Kotlin statics and static extensions Kotlin/KEEP#348
- https://youtrack.jetbrains.com/issue/KT-15595/Optimize-away-unused-and-empty-private-companion-object
- https://youtrack.jetbrains.com/issue/KT-19954/companion-val
- https://youtrack.jetbrains.com/issue/KT-11420
- https://youtrack.jetbrains.com/issue/KT-11968/Research-and-prototype-namespace-based-solution-for-statics-and-static-extensions
Suggested solution
My proposal would be to change logger(func: () -> Unit)
to logger(ref: Any)
. Currently in all cases className derived via reflection (or even via stack trace in js), so it does not matter if it's a lambda or an object. The teams are free to chose what syntax to use then. And also the current code with lambda top-level declaration will work as well.
val topLevelNamedLogger = KotlinLogging.logger("TopLevelNamedLogger")
val topLevelLambdaLogger = KotlinLogging.logger {}
class MyClass {
val classNamedLogger = KotlinLogging.logger("MyClass")
val classLambdaLogger = KotlinLogging.logger {}
val classRefLogger = KotlinLogging.logger(this)
companion object {
val companionNamedLogger = KotlinLogging.logger("MyClassCompanion")
val companionLambdaLogger = KotlinLogging.logger {}
val companionRefLogger = KotlinLogging.logger(this)
}
}
Braking changes
- Logger property delegate for Kotlin/Js was removed. It's not possible to make it work in case of companion object.
- KotlinLogging.logger(underlyingLogger: Logger) was removed due to the clash with object method.
Other changes
- Covered all use cases for Kotlin/js and Kotlin/wasm
- Optimize the KLoggerNameResolver for jvm, since we don't need to search indexOf multiple times