badoo / Reaktive

Kotlin multi-platform implementation of Reactive Extensions
Apache License 2.0
1.18k stars 58 forks source link

Subject wrappers #576

Closed grigorevp closed 3 years ago

grigorevp commented 3 years ago

Since pure RxSwift interop seems not to be available soon, will it be possible to add wrapper classes for subjects?

arkivanov commented 3 years ago

Thanks @Petr-Grigorev, we will consider this and provide updates here.

arkivanov commented 3 years ago

@grigorevp Would it be possible to give some use cases on that?

grigorevp commented 3 years ago

@arkivanov Well, don't really know whether it is a good practice or not, but I like MVVM app architecture. And in my view it is valuable in KMM projects. I was just planning to create view models with some subjects to which user native inputs could be connected and then observed by common logic. Maybe this is an overhead. Moreover, I was able to create some super simple wrappers, providing some subject-like operability for my needs. Not elegant but still. So if you don't see a value for this, it could be closed

arkivanov commented 3 years ago

@grigorevp Using MVVM is totally fine. I would define my view model in Kotlin common code as follows:

interface MyViewModel {
    val title: ObservableWrapper<String>
    val text: ObservableWrapper<String>
    val isButtonEnabled: ObservableWrapper<Boolean>

    fun onButtonClicked()

    fun onTextChanged(text: String)
}

There would be an implementation also in Kotlin common code. Clients can create this view model, subscribe to state updates and call onXxx methods.

grigorevp commented 3 years ago

@arkivanov Sure, this works. Seems I went a bit too reactive, overcomplicating my task

arkivanov commented 3 years ago

Thanks @grigorevp for you input! Closing this for now, unless there are any strong use cases.

grigorevp commented 3 years ago

Can I ask a bit more about your view on reactive MVVM implementation? As soon as we can't push data in Observable directly, we need to find a workaround to use it with some data appearing dynamically. I see two solutions and I don't like both of them.

Firstly, we can use a subject inside the VM and just duplicate it with the observable:

class ViewModel {

    private val someEventSubj = PublishSubject<String>()
    val someEvent = observable<String> {
        someEventSubj.subscribe { next ->
            it.onNext(next)
        }
    }

    fun addEvent(event: String) {
        someEventSubj.onNext(event)
    }
}

Or a more exotic and strange one:

class ViewModel {
    private val someEventEmitterList = mutableListOf<ObservableEmitter<String>>()
    val someEvent = observable<String> {
        someEventEmitterList.add(it)
    }

    fun addEvent(event: String) {
        someEventEmitterList.forEach { 
            it.onNext(event)
        }
    }
}

Am I missing a more legal and beautiful way to do it?

arkivanov commented 3 years ago

@grigorevp What do you think about the following options?

class ViewModel : Relay<String> by PublishSubject() {
    val wrapper: ObservableWrapper<String> = wrap()
}
class ViewModel private constructor(
    subject: PublishSubject<String>
): ObservableWrapper<String>(subject), ValueCallback<String> by subject {
    constructor() : this(PublishSubject())
}
class ViewModel {
    private val subject = PublishSubject<String>()
    val events: ObservableWrapper<String> = subject.wrap()

    fun accept(event: String) {
        subject.onNext(event)
    }
}
grigorevp commented 3 years ago

@arkivanov Yep, some seem quite elegant. The main idea for me here is that we have to use subjects anyway. I hoped there is a way to manipulate observables directly. In case we have a lot of observable properties there will be a lot of subjects also.

P.S. I understand that Subject Wrappers won't do the job. I believe it's quite bad to leave them public, making it possible to push data from view side

arkivanov commented 3 years ago

@grigorevp If I understand correctly, you want to expose only ObservableWrapper and manipulate it internally. If so, then you can just remove the accept method. So clients will only be able to subscribe, and all the manipulation will be implementation details.

grigorevp commented 3 years ago

@arkivanov Yep, this is absolutely clear :) I'm just reflecting that if I have 10 observable properties, there would be 10 corresponding subjects also.

Speaking about wrappers I meant that theoretically if we have them, we can just declare a subject wrapper per property and then use it either to push data from inside or subscribe from outside. But it seems not good as I've said. So thanks for that, I've gotten the idea