Skip to content
This repository has been archived by the owner on Nov 14, 2018. It is now read-only.

Log with lazy messages and default tag parameters #546

Open
RamV13 opened this issue May 18, 2018 · 4 comments
Open

Log with lazy messages and default tag parameters #546

RamV13 opened this issue May 18, 2018 · 4 comments

Comments

@RamV13
Copy link
Contributor

RamV13 commented May 18, 2018

I think it would be extremely useful to extend the android.util.Log API with the functionality of lazy String messages and default tag parameters like so

Log.i("tag") { "msg" }
// or ...
Log.i { "msg" } // where tag = the calling Class' simple name

This could look like

interface Log {
    fun i(tag: String? = null, msg: () -> String): Int
}

val Any.Log: Log
    get() = object : Log {
        override fun i(tag: String?, msg: () -> String) {
            val tag = tag ?: this@Log.javaClass.simpleName
            return if (Log.isLoggable(tag, Log.INFO)) android.util.Log.i(tag, msg()) else 0
        }
    }

This, of course, isn't ideal because we have to introduce an extension property. Even without the default tag parameter, however, due to https://youtrack.jetbrains.com/issue/KT-11968, it would have to be temporarily implemented in some other way besides a static extension function. For example,

interface KTXLog {
    fun i(tag: String, msg: () -> String): Int
}

val Log = object : KTXLog {
    override fun i(tag: String, msg: () -> String): Int {
        return if (Log.isLoggable(tag, Log.INFO)) android.util.Log.i(tag, msg()) else 0
    }
}

This is also clearly not ideal so I'm not sure if this is something that we should wait on or can just implement in one of the two ways as above for now. I'm interested to hear other thoughts on this.

Note: This is similar to #289 but doesn't simplify the API excessively and also leverages the lazy lambdas correctly. On the other hand, this version doesn't get the benefit of inlining.

@romainguy
Copy link
Collaborator

@JakeWharton, what do you think? The proposed extension property above would create a new object on every invocation, which doesn't sound desirable. The lazy message is interesting but wouldn't you end up creating a lambda anyway instead of a String?

@RamV13
Copy link
Contributor Author

RamV13 commented May 24, 2018

The only added benefit of the lazy message in the implementation above would be if the computation for the String is expensive. We could instead implement it as follows and get the inlining back.

object Log {
    inline fun i(tag: String, msg: () -> String): Int {
        return if (Log.isLoggable(tag, Log.INFO)) android.util.Log.i(tag, msg()) else 0
    }
    /* ... */
}

@JakeWharton
Copy link
Contributor

With that you're going to cause confusion with two classes named Log unfortunately.

I'm not sure logging extensions are all that useful in general since log calls tend to be guarded by DEBUG checks so as to not log in production already. And in that case you don't need lazy evaluation since once you clear the conditional you don't really care whether logging is enabled or not. Additionally, developers also use logging abstractions which have other features such as buffering logs in memory for inclusion in crash reporting where you always create the log string but conditionally send it to the actual Android Log.

I'd be open to lazy logging if we could hang static members on the real Log type only because then we're directly enhancing the type with a language feature, but I'm not even convinced they would be used.

@RamV13
Copy link
Contributor Author

RamV13 commented May 24, 2018

Alright then we'll just have to wait on https://youtrack.jetbrains.com/issue/KT-11968

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants