Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make connection bindings work for non Equatable types #8

Closed
d4rkd3v1l opened this issue Mar 11, 2019 · 5 comments
Closed

Make connection bindings work for non Equatable types #8

d4rkd3v1l opened this issue Mar 11, 2019 · 5 comments

Comments

@d4rkd3v1l
Copy link
Contributor

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

@d4rkd3v1l d4rkd3v1l changed the title Make connection bindings working for non Equatable types Make connection bindings work for non Equatable types Mar 11, 2019
@svdo
Copy link
Owner

svdo commented Mar 11, 2019

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
Copy link
Contributor Author

d4rkd3v1l commented Mar 12, 2019

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
Copy link
Owner

svdo commented Mar 12, 2019

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
Copy link
Contributor Author

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
Copy link
Owner

svdo commented Aug 14, 2020

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

@svdo svdo closed this as completed Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants