search

RxSwiftCommunity / contributors

Guidelines for contributing to the RxSwiftCommunity, and a good place to raise questions.
MIT License
21 stars 3 forks source link

RxLifeCycle #32

Open onmyway133 opened 6 years ago

onmyway133 commented 6 years ago

I've read https://github.com/RxSwiftCommunity/contributors and I thought this https://github.com/onmyway133/RxLifeCycle should belong to RxSwiftCommunity. I've used this approach in many of projects, and now it should be a pod, in hope that people find it useful ❤️

The only thing left is to test view controller containment, which I still don't know how to do, here is my question on twitter https://twitter.com/onmyway133/status/931448335625588736, if you know how to test it, please help 🙏

Basically, I would like to avoid swizzling at all cost, and instead use composition, so you can just declare an instance and use it.

Thanks, hope to hear your feedback

freak4pc commented 6 years ago

This comment is entirely personal opinion, so don't take it as a "set in stone" thing. I want to start off by thanking you on the amount of work you spent on the project, as well as the time you spent submitting it to RxSwiftCommunity. It is definitely appreciated ! 😄

First, I think the swizzled approach isn't "nice" - but it is working well since it has been battle tested with various code bases as well as the fact some of the DelegateProxies are using those same methods.

So - generally I'm against swizzling, but in such a battle tested way I live peacefully with it :)

Second, I am usually against having two libraries in RxSwiftCommunity that do the exact same thing unless one of them has a substantial benefit over the other, or the other one has been heavily under-maintained. This doesn't seem to be the case here, and @devxoul's RxViewController seems to be a popular, useful and maintained choice.

Lastly, I'm not a huge fan of the API you describe here. Having a wrapper object just to get Reactive Observation seems a bit excessive to me. It also breaks the "discovering" syntax of RxCocoa (e.g. item.rx.propety) by adding a custom .rxLifeCycle namespace.

Overall, my personal opinion is we don't need another life cycle project in RxSwiftCommunity, but I'm 100% open to any opinions on this and I'm definitely not religious to it.

bobgodwinx commented 6 years ago

@onmyway133 Can you please checkout this repo https://github.com/devxoul/RxViewController it looks like what you're doing is already available and well tested. You can also add your contribution to it and I would ask @devxoul to bring in all his @RxSwift related repos into the @RxSwiftCommunity in order to avoid further confusion.