Better replace logger support
I don't think it's safe to call
1. LogcatLogger.uninstall()
1. LogcatLogger.install(AndroidLogcatLogger())
Because another thread could call logcat { "Well intended message" } between lines 1 and 2.
It's true that uninstall sets the logger to NoLog which says nothing is loggable. This would likely just drop any logcat{} logs that come in on the floor.. but... I was thinking that since the logcat{} calls delegate to the logger without locking on Logcat (which is an obvious choice and makes sense, you don't want to lock on that), but since we don't, I think it could be (looking at the implementation)
inline fun Any.logcat(
priority: LogPriority = DEBUG,
/**
* If provided, the log will use this tag instead of the simple class name of `this` at the call
* site.
*/
tag: String? = null,
message: () -> String
) {
LogcatLogger.logger.let { logger ->
// IF THE LOG SWAP TO NoLog HAPPENS AFTER THIS if isLoggable LINE FIRES
if (logger.isLoggable(priority)) {
val tagOrCaller = tag ?: outerClassSimpleNameInternalOnlyDoNotUseKThxBye()
// BUT BEFORE THIS logger.log LINE FIRES
logger.log(priority, tagOrCaller, message())
}
}
}
Then the NoLog will throw an exception
private object NoLog : LogcatLogger {
override fun isLoggable(priority: LogPriority) = false
override fun log(
priority: LogPriority,
tag: String,
message: String
) = error("Should never receive any log")
}
}
tl;dr;
I think we should offer a replaceLogger something like..
fun replaceLogger(newLogger: LogcatLogger) {
synchronized(this) {
uninstall()
install(newLogger)
}
}
And I think we should make NoLog gentler and something like
private object NoLog : LogcatLogger {
override fun isLoggable(priority: LogPriority) = false
override fun log(priority: LogPriority, tag: String, message: String) = Unit
}
}
If you need to have two logger implementations and swap these out, you can just make a logger implementation that does that within itself.
The API was never meant to be used as "uninstall(), install()" as a way to swap impls, and I'd rather not relax NoLog just for a use case that the APIs aren't meant to support and that AFAIK can be implemented with a custom logger