make Reducer a fun interface?

[badoo]

make Reducer a fun interface?

[arkivanov]

This is because I want all changes to be reasonable.

[arkivanov]

This sounds like a good idea! Would you like to submit a PR?

[dalewking]

You want a PR for just adding the keyword "fun" in front of the interface?

[arkivanov]

Well yes. I can do it by myself probably later this week.

[dalewking]

While you're at it ViewRenderer should also be a fun interface

[arkivanov]

I would keep ViewRenderer as it is unless there is a use case.

[dalewking]

What is the use case for NOT making ViewRenderer a fun interface. Basically any interface with a single abstract non-suspend method should probably be a fun interface unless there is a compelling reason not to

The use case is that I am not using MviView mostly because I don't want to use ViewEvents on Android since I am using FlowBinding library to create a Flow directly without all the boiler plate. So I deal with a ViewRender directly, but even if using BaseMviView it has a field where you can set a ViewRenderer.

If I don't use diffing I have to do something like this to create a ViewRenderer:

         val renderer = object: ViewRenderer<State> {
            override fun render(model: State) {
                label.text = model.value
            }
        }

Adding fun to ViewRenderer lets you simplify to

         val renderer = ViewRenderer<State> { label.text = it.value }

Your reaction implies to me that you may not fully understand the benefits of a fun interface. There is a great article on the subject.

[arkivanov]

I think in your case you can just use normal functions, so your renderer becomes just a consumer of ViewModels: (Model) -> Unit.
Regarding BaseMviView: you can just override its render method, no need to assign the renderer property.
The ViewRenderer interface is not meant to be created anonymously, but if you think it makes sense then let's make it fun interface as well.

[dalewking]

I think in your case you can just use normal functions, so your renderer becomes just a consumer of ViewModels: (Model) -> Unit.

Except that is not type compatible with diff function

Regarding BaseMviView: you can just override its render method, no need to assign the renderer property.

But you allow for it so someone could want to use the property

The ViewRenderer interface is not meant to be created anonymously, but if you think it makes sense then let's make it fun interface as well.

Which was my point because I do want to create it anonymously

[arkivanov]

Could you kindly provide a code snippet? From my point of view, you either use diff or normal Kotlin functions. In neither case you need to create ViewRenderer anonymously.

[dalewking]

So in my case I have no MviView implementation class. The controller onViewCreated looks like this:

fun onViewCreated(
    viewLifecycle: Lifecycle,
    renderer: ViewRenderer<State>,
    intents: Flow<Intent>,
    eventHandler: suspend (Event) -> Unit
) {
    bind(viewLifecycle, BinderLifecycleMode.CREATE_DESTROY) {
        intents bindTo store
        store.states bindTo renderer
        store.labels bindTo eventHandler
    }
}

In my Android fragment I have:

        val intents = merge(
            button1.clicks().map { SomeIntent },
            button2.clicks().map { AnotherIntent },
            ...
        )

        val renderer = object: ViewRenderer<State> {
            override fun render(model: State) {
                label.text = model.data
            }
        }

And then to call the controller I have:

        controller.onViewCreated(
            viewLifecycleOwner.lifecycle.asMviLifecycle(), renderer, intents
        ) {
            // handle labels here
        }

So I could switch the controller parameter to just take (Model) -> Unit and change the renderer field to just be a lambda BUT if I decided later that the renderer did need to use diff I would also have to switch the controller to ViewRenderer. So it is best to stay with ViewRenderer so the Controller doesn't know the difference.

Making ViewRenderer a functional interface means I can use this to declare the renderer:

        val renderer = ViewRenderer<State> { label.text = it.data }

and if I want to switch renderer to diff I can say:

        val renderer = diff { diff(State::data) { label.text = it } }

without changing the Controller at all

[arkivanov]

Thanks, that is a valid use case indeed. Will love to make ViewRenderer a fun interface.

[dalewking]

That sure was a lot of discussion to get a total of 4 characters added to a file

[arkivanov]

Looks like marking an interface as fun may be a breaking change.

For example, the following case fails with StackOverflowError after making ViewReneder a fun interface:

    override fun <Model : Any> Observable<Model>.bindTo(viewRenderer: ViewRenderer<Model>) {
        this bindTo {
            assertOnMainThread()
            viewRenderer.render(it)
        }
    }

This function becomes recursive once ViewRenderer is marked as fun, because now this lambda is treated as ViewRenderer due to SAM conversion.

I think I will keep ViewRenderer as it is for now. You can add the following function to reduce the boilerplate:

inline fun <T : Any> viewRenderer(crossinline render: (T) -> Unit): ViewRenderer<T> =
    object : ViewRenderer<T> {
        override fun render(model: T) {
            render.invoke(model)
        }
    }