Kotlin / anko

Pleasant Android application development
Apache License 2.0
15.9k stars 1.28k forks source link

I don't understand the purpose of runOnUiThread() and some other naming conventions... #728

Open tprochazka opened 5 years ago

tprochazka commented 5 years ago

Anko is an interesting library, but I don't understand the purpose of runOnUiThread() method defined on support Fragment by Anko. It brings just confusion to users of the library. It always crashes when the user leaves the fragment before it finish, you need always use supportFragmentUiThread or activityUiThread which is quite unclear for the user of the library. Also, many tutorials mention just uiThread.

And I don't understand why there is supportFragmentUiThread and fragmentUiThread, it can have the same name for both type of fragment, right? Both are defined in a different class, there is no way to use both in one.

Also would be much easier if supportFragmentUiThread or activityUiThread will be named uiThreadFragment and uiThreadActivity because AS will automatically suggest it when you will write uiThread inside of doAsync. uiThread itself makes sense just outside of fragment of activity.

christopheblin commented 5 years ago

I completely agree with this issue : the documentation of anko coroutines is awful and in the end you never know what to do or if what you do is safe (in regard to the android lifecycles).

For ex, are the following code snippets 1. safe ? 2. equivalent ? :

doAsync {
    val infos = model.getInfos(offerId!!)
    fragmentUiThread { onInfos(infos) }
}

Versus

onInfos(doAsyncResult { model.getInfos(offerId!!) }.get())
//here I suppose that onInfos must check for detached state as well as activity.isFinishing ?

Please note that this is not a bash toward anko coroutines !

I find it way more clearer than any RxJava or Future or ..., it is just the documentation that is outdated and does not provide a real best practice

As a side note, I am using the first code snippet (I hope it is the good one :))

tprochazka commented 5 years ago

@christopheblin Here is quite obvious that onInfos(doAsyncResult { model.getInfos(offerId!!) }.get()) is wrong, because get() immediatelly block the main thread until background thread finish it's job. So it is equivalent of onInfos(model.getInfos(offerId!!))