Issue #484 - Optimize the KLoggerNameResolver for jvm

[scrat98]

closes #484

[oshai]

I am not sure what is the optimization here?

[scrat98]

@oshai it's unnecessary to search the same delimiter twice

1. contains = indexOf(delimeter) >= 0
2. substringBefore = substring(0, indexOf(delimeter))

[scrat98]

@oshai also please note, that there is a problem with a tests that're not covering name(func: () -> Unit): String at all. The tests cover name(forClass: Class<T>) only, that have completely another logic to obtain the logger name #484 (comment)

[oshai]

@oshai it's unnecessary to search the same delimiter twice

1. contains = indexOf(delimeter) >= 0
2. substringBefore = substring(0, indexOf(delimeter))

got it.

I am not sure wdym by tests not covering func. Anyway, it will be nice if you'll add unit test to the changed class.

[scrat98]

@oshai I'll update the tests, but they will rely on current behavior. For example, tests expected to support inner classes

Arguments.of("io.github.oshai.kotlinlogging.internal.Foo\$Bar", Foo.Bar::class.java)

But currently Inner classes are not supported already, therefore it may seems that my changes break behavior, but actually not, this behavior was already changed.

You can try this with the latest library version

class Base {
  class Inner {
    private companion object {
      private val logger = KotlinLogging.logger { }
    }

    init {
        logger.info { "Hello" }
    }
  }
}

fun main() {
  Base.Inner()
}

18:28:14.314 [main] INFO  io.github.oshai.kotlinlogging.internal.Base - Hello

Please, see the comment here: #484 (comment)

[scrat98]

@oshai please have a look

[oshai]

I think this method should be private

[scrat98]

@oshai historically this method was used in the tests, therefore it's not possible to make it private without rewriting tests structure. https://github.com/oshai/kotlin-logging/blob/master/src/javaMain/kotlin/io/github/oshai/kotlinlogging/internal/KLoggerNameResolver.kt#L21

I would prefer to keep it internal for now and if the changes from #481 will be accepted, then signature of the method will be changed from

internal actual fun name(func: () -> Unit): String

to

internal actual fun name(ref: Any): String {

and then we can update the tests to use instances of the classes, rather then KClass

[oshai]

ok. checks are now failing - I think on code formatting.

[scrat98]

Applied formatting, I'm sorry, completely forgot about it

[scrat98]

Also I think it's worth to mention somewhere in the documentation that loggers in the inner classes always will have the name of the parent(base) class. Example from here #485 (comment)

Or create another issue if it supposed to be fixed

[scrat98]

Created issue about logger name for inner class #495

[oshai]

Thanks for the PR!