bernaferrari / ChangeDetection

Automatically track websites changes on Android in background.
Apache License 2.0
709 stars 99 forks source link

Callbacks and coroutines? #10

Closed dmytrodanylyk closed 6 years ago

dmytrodanylyk commented 6 years ago

Is there a reason why you have callbacks in your repositories and data source classes and use coroutines in your ViewModel?

I suggest making all public fun of repositories suspended. See this example.

bernaferrari commented 6 years ago

Is there a reason why you have callbacks in your repositories and data source classes and use coroutines in your ViewModel?

Yes! Lack of knowledge! I have only a few weeks from experience with coroutines (and a few months with rxjava), so these things are still kind of new and sometimes hard for me. The coroutines/suspend functions inside the app were supposed to simplify the cases where callbacks became unnecessary.

However.. after realising you did the hard work converting them, I went ahead and just pushed the app for full coroutines (totally inspired by your app, I had no idea the withContext(appExecutors.ioContext) could replace those ugly AppExecutors).

So, consider it done! https://github.com/bernaferrari/ChangeDetection/commit/f2742df121e3f84dedd35f2699851032aa2bc6a7

Thanks for the feedback. If you have any other comment, suggestion or feedback, I am open!

dmytrodanylyk commented 6 years ago

👍

bernaferrari commented 6 years ago

@dmytrodanylyk I am having the following regression since the full coroutines migration:

https://www.reddit.com/r/androiddev/comments/8rylh6/weekly_questions_thread_june_18_2018/e0z20bj

Any ideas on how to fix it?