android-ktx icon indicating copy to clipboard operation
android-ktx copied to clipboard

Alternative API for creating bundles

Open gabrielhuff opened this issue 7 years ago • 5 comments

The current bundle creation method (bundleOf) accepts an array of Pair<String, Any?>. Although the implementation takes care of resolving the value type at runtime, the API is 'unsafe' at compile time (anything can be added as a value).

I propose a new API, consisting of a simple DSL, that offers compile-time safety. Something like:

val bundle = bundle {
  
  "key1" to 42         // Resolved to Int at compile time
  
  "key2" to 42L        // Resolved to Long at compile time
  
  "key3" to "42"       // Resolved to String at compile time

  "key4" to Fragment() // Fails at compile time
}

The bundle method would receive a configuration block to be delegated to a builder object (an interface), something like:

fun bundle(configure: BundleBuilder.() -> Unit): Bundle {
  return BundleBuilderImpl().apply(configure).bundle
}

interface BundleBuilder {
  
  infix fun String.to(value: Int)
  
  infix fun String.to(value: Long)

  // ...
}

private class BundleBuilderImpl : BundleBuilder {
  
  val bundle = Bundle()
  
  override fun String.to(value: Int) { bundle.putInt(this, value) }
  
  override fun String.to(value: Long) { bundle.putLong(this, value) }

  // ...  
}

Maybe even add a sub-bundle operator:

val bundle = bundle {
  
  "key1" to 42          // Resolved to Int at compile time
  
  "sub_bundle" {        // Equivalent to "sub_bundle" to bundle { ... }
    
    "sub_key1" to 42.0f // Resolved to Float at compile time
  }
}

interface BundleBuilder {
  
  // ...

  operator fun String.invoke(configure: BundleBuilder.() -> Unit)

  // ...
}

private class BundleBuilderImpl : BundleBuilder {
  
  val bundle = Bundle()
  
  // ...  
  
  override operator fun String.invoke(configure: BundleBuilder.() -> Unit) {
    bundle.putBundle(this, bundle(configure))
  }

  // ...  
}

gabrielhuff avatar Feb 07 '18 17:02 gabrielhuff

Something like this might be feasible once we have inline classes in Kotlin 1.3 and can make this zero-overhead. I'd rather see a lint check for now guard against types which cannot be placed in a bundle.

The sub-bundle handler seems entirely superfluous as "sub_bundle" to bundle { already would work.

JakeWharton avatar Feb 07 '18 18:02 JakeWharton

I apologize, but would you mind clarifying how this would correlate to inline classes?

About the sub-bundle, it's just my opinion, but it'd make the code visually cleaner (anything that's not to would be either a key or value)

gabrielhuff avatar Feb 07 '18 18:02 gabrielhuff

The forthcoming inline classes prevents the need for the wrapper class to be allocated around a Bundle and allows all the functions to be inline. This means that it adds zero overhead compared to traditional bundle creation. If we're going to abandon parity with the other map-like factories I'd be nice to get some tangible benefits like this. DSLs are currently very leaky in terms of allocation and the types and functions they put into the resulting APK.

JakeWharton avatar Feb 07 '18 18:02 JakeWharton

Alright, appreciate the answer

gabrielhuff avatar Feb 07 '18 19:02 gabrielhuff

I honestly think bundleOf in general is a mistake.

Sure it auto-resolves the put* part, but now how do I know which one it defaulted to when I want to get* it?

Even Bundle().apply { is more explicit in its usage.

Zhuinden avatar Apr 07 '18 23:04 Zhuinden