Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Unnecessary async extension functions? #4

Closed
robertoestivill opened this issue May 6, 2019 · 1 comment
Closed

Unnecessary async extension functions? #4

robertoestivill opened this issue May 6, 2019 · 1 comment

Comments

@robertoestivill
Copy link

robertoestivill commented May 6, 2019

I am reading the code out of curiosity and was just wondering about this piece of code

fun Any.async(c: suspend AsynkHandler.() -> Unit): AsynkHandler {
    val controller = AsynkHandler(this)
    keepCoroutineForCancelPurpose(controller)
    return async(c, controller)
}

fun Activity.async(c: suspend AsynkHandler.() -> Unit): AsynkHandler {
    val controller = AsynkHandler(this)
    keepCoroutineForCancelPurpose(controller)
    return async(c, controller)
}

fun Fragment.async(c: suspend AsynkHandler.() -> Unit): AsynkHandler {
    val controller = AsynkHandler(this)
    keepCoroutineForCancelPurpose(controller)
    return async(c, controller)
}

fun android.support.v4.app.Fragment.async(c: suspend AsynkHandler.() -> Unit): AsynkHandler {
    val controller = AsynkHandler(this)
    keepCoroutineForCancelPurpose(controller)
    return async(c, controller)
}

Wouldn't the first extension function just work for all the other definitions since it's an extension function of Any ? Why override it per activity/fragment?

fun Any.async(c: suspend AsynkHandler.() -> Unit): AsynkHandler {
    val controller = AsynkHandler(this)
    keepCoroutineForCancelPurpose(controller)
    return async(c, controller)
}

Would you consider a PR to remove them ?

Thanks!

@CuriousNikhil
Copy link
Owner

Hi, thanks for the suggestion.
Yes. I was experimenting with the Any.async(...)
In activity/Fragment when you start coroutine block with async{...}, you've to take care of the memory leaks so you have to use async.cancelAll(). That code I let for experiment and Was trying to port this one for non-android platforms as well. Yes sure you are right I'll rather remove the Any.async() instead of other extensions of async. OR, There's one possible solution to register viewLifeCycleObserver with async so that in onDestroy() we could cancel it all internally and user is free from lifting that cleanup stuff. You can raise a PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants