svdo / ReRxSwift

ReRxSwift: RxSwift bindings for ReSwift
https://svdo.github.io/ReRxSwift
MIT License
99 stars 16 forks source link

Make connection bindings work for non Equatable types #8

Closed d4rkd3v1l closed 4 years ago

d4rkd3v1l commented 5 years ago

Sometimes I need to use a "protocol type" as a prop. Unfortunately, there is an Equatable constraint on the types, that can be used to subscribe/bind. There fore this is only possible going down the "type erasure road", afaik. But this is just completely overkill for me here.

So I would suggest, adding additional binders without the Equatable constraint, but instead enforce to supply a manual isEqual function. (like you actually do, one step deeper^^)

For testing I just modified the one of the existing binders:

    public func bind<S: Sequence,M>(_ keyPath: KeyPath<Props, S>,
                                    to binder: (Observable<M>) -> Disposable,
                                    isEqual: @escaping (S,S) -> Bool,
                                    mapping: ((S)->M)? = nil)
    {
        let distinctAtKeyPath = self.propsEntry(at: keyPath) { isEqual($0, $1) }

        let afterMapping: Observable<M>
        if let mapping = mapping {
            afterMapping = distinctAtKeyPath.map(mapping)
        } else {
            afterMapping = distinctAtKeyPath as! Observable<M>
        }

        afterMapping
            .bind(to: binder)
            .disposed(by: disposeBag)
    }

What do you think? :-) If you like it, I can create a Pull request for this, but I just wanted to ask first, before I put more work into this.

Thanks in advance Martin

svdo commented 5 years ago

Thanks for discussing this first, that's always so much nicer :)

I'm still working on wrapping my head around "type erasure". I didn't run into this problem with ReRxSwift and I'm not sure what causes you to have the problem. I just watched this great talk by Rob Napier: https://www.dotconferences.com/2016/01/rob-napier-beyond-crusty-real-world-protocols. Any solution directions for you in there maybe?

Having said all that, I'm not opposed to adding a variant with an isEqual parameter, as long as everything else keeps working the way it currently does :) In other words: I don't want to require an isEqual parameter...

If you do end up creating a pull request, could you kindly include also unit tests and doc updates? Thanks!

d4rkd3v1l commented 5 years ago

Thanks for your fast answer.

I must admit, protocols are also not my biggest strength, as well ;-)

But afaik it's just not possible to make a protocol conform to Equatable, as it "has Self requirements". https://stackoverflow.com/a/24890041/2019384

I think there actually is a sense in this, as you don't know what the actual classes/structs that conform to your protocol will look like and you can not really ensure equatability on this (general) level. But somehow this just works fine in other languages like C# afair. 0_o

However that's why it sometimes is really handy/necessary to be able to use a custom "isEqual"-like function.

I would actually add additional functions for that, without the Equatable constraint, but the custom isEqual closure instead. Sure, I won't mess in your code too much^^ #docs #tests #code-style

svdo commented 5 years ago

Yeah I recommend watching that video I mentioned if you didn't yet. Rob explains some common architecture patterns that you can use to avoid this problem. And he also mentions that he hopes that the Swift compiler will take care of this for you some day :)

But again, feel free to make that pull request!

d4rkd3v1l commented 5 years ago

Yea that's a really interessting talk. So basically instead of using these crippled protocols, try generic structs. I'll definitely give it a try. If it doesn't work you'll see a PR in the near future 😉

svdo commented 4 years ago

This is quite a while ago now, so I'm assuming this is no longer an issue. Please reopen if needed.