Add 'event' parameter to 'handled' lambda in 'ViewEvent' 'peek()'

[eugenio1590]

This is not an important change, but It's an enhancement. Currently, the function peek, It's the following:

viewModel.observeState(this) {
    it.event?.peek {
        when (it.event) {
            is CatViewEvent.Meow -> {
                // show Meow! dialog
                true
            }
        }
    }
}

But into the peek function, I need use again the "it.event". It's can resolve changing the parameter in the peek function:

fun peek(handled: ViewEvent.() -> Boolean) {
    if (!alreadyHandled) alreadyHandled = this.handled()
}

So, the new peek method would be used:

viewModel.observeState(this) {
    it.event?.peek {
        when (this) {
            is CatViewEvent.Meow -> {
                // show Meow! dialog
                true
            }
        }
    }
}

[eugenio1590]

Or if you prefer to use "it" instead the "this":

fun peek(handled: (ViewEvent) -> Boolean) {
    if (!alreadyHandled) alreadyHandled = handled(this)
}

viewModel.observeState(this) {
    it.event?.peek {
        when (it) {
            is CatViewEvent.Meow -> {
                // show Meow! dialog
                true
            }
        }
    }
}

[etiennelenhart]

Hey, thanks for the feedback.

While developing the new 'peek' style handling I actually planned to provide the event as a parameter to the handled lambda. So your second example would have been possible.

One problem I encountered though, is that the parameter wouldn't be specifically typed then. (see #30) So the it would be of type ViewEvent instead of CatViewEvent and therefore break the exhaustive when expression.

If you have a solution to this, I'd be more than happy to include it.

[eugenio1590]

Umm you can try with T parameter and inline function. I am not sure about this resolve the problem. But you can try.

abstract class ViewEvent {
    var alreadyHandled = false
}

inline fun <ViewEvent> T.peek(handled: (T) -> Boolean) {
    if (!alreadyHandled) alreadyHandled = handled(this)
}

The problem with this is that the alreadyHandled variable is public.

[etiennelenhart]

Sure, an extension would work since it's basically a static function. I'm just not sure I'm comfortable making alreadyHandled public. It would make the API to handle a ViewEvent a bit ambiguous.

[etiennelenhart]

I gave it some thought and went with your proposal of the extension function. ViewEvent now exposes a mutable handled property and peek() provides the current event as a parameter to the handled lambda. The change is already live in 4.0.1.

[eugenio1590]

Hi @etiennelenhart , I apologize for the delay. Thanks for the support. I was thinking about it too and I think that the public variable would help to execute events again. Similarly, the visibility "internal" would help a lot. Thanks again for the support, I will be testing as soon as possible.

[etiennelenhart]

Normally executing an event multiple times should be accomplished by setting an updated ViewState with a new event.

I totally forgot about internal. That should accomplish pretty much exactly what I'm aiming for. I'll probably keep this for a later update though since it's a really minor thing.

Thanks for your input. 🙂