android / android-ktx

A set of Kotlin extensions for Android app development.
https://android.github.io/android-ktx/core-ktx/
7.47k stars 564 forks source link

Intent builder #166

Open gildor opened 6 years ago

gildor commented 6 years ago

Kotlin stdlib uses lambdas to build different types of complex objects: buildString buildSequence List builder

I propose to add intent builder to ktx. That will work like:

buildIntent {
   action = "some.Action"
   data = "http://example.com".toUri()
   putExtra("name", value)
}

Possible implementation:

inline fun buildIntent(initializer: Intent.() -> Unit) = Intent().apply { initializer() }

Also we can move often used properties to arguments of the function, such as action:

inline fun buildIntent(action: String) = Intent(action).apply { initializer() }

but not sure that this is make a lot of sense.

But another feature would more useful. Intent builder with reified generic instead class argument:

inline fun <reified T> buildIntent(
        context: Context, 
        initializer: Intent.() -> Unit
) = Intent(context, T::class.java).apply { initializer() }

This version also can provide empty lambda as default argument for initilizer, to make builder usage optional, reified acitivity creation already useful feature

Possible naming:

Intent (same as List builder, that looks like constructor call but actually function) buildIntent (same as stringBuilder) intent is short but doesn't follow current Kotlin naming and clashes with variable names

JakeWharton commented 6 years ago

Dupe of #154.

JakeWharton commented 6 years ago

Actually I'm going to dupe that one onto this as this proposal is a lot closer to the prototype I have.

gildor commented 6 years ago

One more thing: To make activity start even easier we can consider adding buildIntent extension function to Activity and Fragment with a different name just to start activities with optional initializer (the most often case): Possible implementation:

inline fun <reified T : Activity> Activity.startActivity(
        initializer: Intent.() -> Unit = {}
) {
    val intent = Intent(this, T::class.java).apply { initializer() }
    startActivity(intent)
}

Also, make sense to have startActivityForResult extension

There is also an option to add extension function to Context, but this approach is not safe because a user can try to start activity from application context without FLAG_ACTIVITY_NEW_TASK and will get runtime error

If it makes sense I can create a separate feature request.

dovahkiin98 commented 6 years ago
inline fun <reified T : Activity> Context.startActivity(vararg args: Pair<String, Any>) {
    val intent = Intent(this, T::class.java)
    intent.putExtras(bundleOf(*args))
    startActivity(intent)
}
peikedai commented 6 years ago

One of the main inconvenience when you need to pass Parcelable to the new Acitivity is that the name for the "extra data" needs to be maintained somewhere as a String constant. And both the starter and the starting Activity need to know that name while what both sides only need is the "extra data"

My proposal to get ride of that String constant is using this:

inline fun <reified T : Activity> Context. newIntent(obj: Parcelable): Intent {
    val intent = Intent(this, T::class.java)
    val name = getExtraName(T::class.java)
    intent.putExtra(name, obj)
    return intent
}

// the name will be something like: com.your.package.YourCoolActivity.ExtraName
fun getExtraName(target: Class<out Activity>): String {
    return "${target.canonicalName}.ExtraName"
}

And if you have startActivity like this:

inline fun <reified T : Activity> Context.startActivity(obj: Parcelable) {
    val intent = newIntent<T>(obj)
    this.startActivity(intent)
}

Then the usage of that will be super simple:

startActivity<YourCoolActivity>(theParcelable)

To get the "extra data", you just need to create another extension function:

inline fun <reified T : Parcelable> Activity.getParameter(): T {
    val name = getExtraName(this.javaClass)
    val obj = intent.getParcelableExtra<T>(name)
    return obj
}

This is how to use it from YourCoolActivity:

val data = getParameter<TheParcelable>()
gildor commented 6 years ago

@dovahkiin98 I don't like your proposal with vararg pair because of many reasons:

  1. You create one additional object on each argument + array on each call
  2. Not a big improvement comparing with standard api + initializer
    startActivity<SomeActivity>(
    "foo" to "bar",
     "theAnswer" to 42
    )
    // vs
    startActivity<SomeActivity> {
    putExtra("foo", "bar")
    putExtra("theAnswer", 42)
    )

    Also initilizer allow to write much more complicated code:

    startActivity<SomeActivity> {
    if (bar != null) putExtra("foo", bar)
    putExtra("theAnswer", 42)
    )
  3. Your solution is not type safe. You cannot put Any to bundle, only Parcelabel/Serializable/primitive types, so not sure how bundleOf should work, but it cannot force user to pass only Parcelabel/Serializable types
dovahkiin98 commented 6 years ago

@gildor Why can't they both be added? each one offers a different implementation. If you want to only add extras, you can use the one with vararg, and if you want to be more specific about the initialization, you can use the one with the lamba.

The method bundleOf is already in the API

fun bundleOf(vararg pairs: Pair<String, Any?>)

it checks if the type can be added to a bundle or not, and throws exceptions on any error. Maybe something like this:

inline fun <reified T> Context.startActivity(vararg args: Pair<String, Any?>, init: Intent.() -> Unit = {}) {
    val intent = Intent(this, T::class.java)
    intent.putExtras(bundleOf(*args))
    intent.apply(init)
    startActivity(intent)
}

startActivity<MainActivity>("Number" to 1) {
    addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
}
LouisCAD commented 6 years ago

Here's what I've been using for starting Activities:

inline fun <reified A : Activity> Context.start(configIntent: Intent.() -> Unit = {}) {
    startActivity(Intent(this, A::class.java).apply(configIntent))
}

inline fun Context.startActivity(action: String, configIntent: Intent.() -> Unit = {}) {
    startActivity(Intent(action).apply(configIntent))
}

It is used like so:

start<AboutActivity>()

startActivity(Settings.ACTION_APPLICATION_DETAILS_SETTINGS) {
    data = Uri.fromParts("package", packageName, null)
}

I used start over startActivity for the first one because "Activity" is already present in the name of any Activity who's naming convention follows Android recommendations.

objcode commented 6 years ago

Lots of good ideas in this thread. A use case to keep in mind while designing this is Intents used by testing, e.g. the example on this page: https://developer.android.com/reference/android/support/test/espresso/intent/Intents.html

damianw commented 6 years ago

Personally I would love to see intentFor combining both ideas from startActivity and bundleOf, like Anko has:

inline fun <reified T: Any> Context.intentFor(vararg params: Pair<String, Any?>): Intent {
    return Intent(context, T::class.java).apply { putExtras(bundleOf(*params)) }
}
Zhuinden commented 6 years ago

Personal opinion, but while we use bundleOf("x" to y) at work, I've never liked it. You can't be sure how exactly it builds the Bundle, and what getter you have to call to retrieve your object. 😞 The Intent.() -> Unit builder is much cleaner.

Ioane5 commented 6 years ago

Probably the cleanest would be if each Activity had to receive Parameters Object (With it's all attributes). Bundle is too vague and unsafe.

Zhuinden commented 6 years ago

@Ioane5 You're actually correct, you can use a @Parcelize data class

LouisCAD commented 6 years ago

@Ioane5 I wrote a small library for typesafe Bundles, without any annotation processing, just plain Kotlin. Here's an example: https://github.com/LouisCAD/Splitties/tree/72a7faef07ae8ad8918c5c5f4d1f9c0ff3aad0f3/intents/README.md#example

I doubt this would be integrated into Android KTX because it's a bit more than just extensions, but it is an alternative to SafeArgs (currently used for Navigation Architecture Components) from AndroidX, that is IMHO easier to setup, lighter and more versatile (works for Activities, Fragments, Services, BroadcastReceivers, saved instance state, and anything else that uses a Bundle if you look at the fragmentargs and bundle modules).

Ioane5 commented 6 years ago

@LouisCAD I forgot about SafeArgs at all.

I do like your approach, and clearly It's better than plain bundle, since you set parameter without string key. But as I see, you cannot enforce parameters by using

        start(DemoActivity) { intentSpec, extrasSpec -> 
            extrasSpec.showGreetingToast = isUserPolite // <-- clumsy me forgot this param
            extrasSpec.optionalExtra = intentSpec.someText
        }

What if user does not specify required parameter showGreetingToast? Will there be any compile error? I think no.

With SafeArgs I hope compile time error is generated.

LouisCAD commented 6 years ago

@Ioane5 This currently results into a runtime error, but in case of Fragments, it could be catched earlier with Android lint as required arguments are the ones delegated by arg() (and not argOrNull, argOrElse or argOrDefault). An annotation to mark BundleSpec properties as required could make up for it for non Fragment usages so lint can detect programmer errors there too. I currently have no experience with lint though, but you are into it, open an issue about it!

BTW, thanks to autocomplete on extrasSpec in the example you showed, discovery is improved, which makes you much less likely to forget an intent extra, so I don't think it's so important.