etiennelenhart / Eiffel

Redux-inspired Android architecture library leveraging Architecture Components and Kotlin Coroutines
MIT License
211 stars 14 forks source link

Log output for an Interception class name #78

Closed jordond closed 5 years ago

jordond commented 5 years ago

Something else I didn't catch in #72 or #77

So currently if you create a Interception using one of their helper functions, ie: pipe {...}. Then inspect the logs when that interception is applied, you will see a blank space where it's name should be.

ex:

image

And that is because we are calling this::class.java.simpleName to get the name, but according to this simpleName will return an empty string if called on an anonymous object. Something which I was unaware of.

So I've thought of one simple solution, but I am open to hearing your thoughts on solving this problem.

  1. Replace this::class.java.simpleName with this.javaClass image

  2. Call the default toString method of the interception.

    • Will return the same result as above
  3. Override the toString method, default to the name of the interception, or the user can override it to rename it something different.

    • Default:

      
      abstract class Pipe<S : State, A : Action> : Interception<S, A> {
      protected open val name: String = ""
      ...
      
      override fun toString(): String = if (name.isEmpty()) super.toString() else name
      }

    fun <S : State, A : Action> pipe( name: String = "", before: (state: S, action: A) -> Unit = { , -> }, after: (state: S, action: A?) -> Unit = { , -> } ): Pipe<S, A> { return object : Pipe<S, A>() { override val name: String = name

        override fun before(state: S, action: A) = before(state, action)
    
        override fun after(state: S, action: A?) = after(state, action)
    }

    }

    Then use it:
    
    ```kotlin
    pipe {...}
    // ├─ ↓ Interception: com.etiennelenhart.eiffel.interception.PipeKt$pipe$3@c4f6873
    
    pipe("SomeAwesomePipe") {...}
    // ├─ ↓ Interception: SomeAwesomePipe
    • Maybe even suffix the default toString with the name
      • ie: com.etiennelenhart.eiffel.interception.PipeKt$pipe$3@c4f6873:SomeAwesomePipe

It might be useful if we give the user the availability to modify the name of their interceptions for easier debugging.

Obviously 1, and 2 are the easiest to implement. But I also like the possibility of 3. Because if you have 5+ of the same interception, it would be nice to differentiate between them.

So let me know your thoughts, and I can create a PR. If you have any other ideas, or thoughts, let me know!

etiennelenhart commented 5 years ago

Shoot, I guess this also slipped through since the Interception objects in the EiffelViewModel unit test are stored in properties and their names get properly printed... And I thought logging would be simple. 🙂 I'll think about it and get back to you tomorrow.

etiennelenhart commented 5 years ago

I'd also like to go for option 3 just a bit more simplified.

I think we could add a debugName property directly to the Interception interface like this:

interface Interception<S : State, A : Action> {

    val debugName: String
        get() = toString()

    suspend operator fun invoke(scope: CoroutineScope, state: S, action: A, dispatch: (action: A) -> Unit, next: Next<S, A>): A?
}

Implementing classes may then override debugName but we don't have to modify the existing implementations like Adapter, Pipe, etc. just the builder functions as you proposed:

fun <S : State, A : Action> pipe(
    debugName: String = "",
    before: (state: S, action: A) -> Unit = { _, _ -> },
    after: (state: S, action: A?) -> Unit = { _, _ -> }
): Pipe<S, A> {
    return object : Pipe<S, A>() {
        override val debugName = debugName.ifEmpty { toString() }

        override fun before(state: S, action: A) = before(state, action)

        override fun after(state: S, action: A?) = after(state, action)
    }
}

In EiffelViewModel we could simply change the log to use debugName and we should be good to go:

log("├─ ↓ Interception:  $debugName")
jordond commented 5 years ago

Sounds good! I'll start it tomorrow.