pocorall / scaloid

Scaloid makes your Android code easy to understand and maintain.
Other
2.09k stars 163 forks source link

Add implicit conversions to Parcelable #88

Closed RyanGlScott closed 10 years ago

RyanGlScott commented 10 years ago

I frequently use Android's Parcelable interface to allow for my classes to be transported across Activity changes (e.g., screen rotations or passing via Intents). Android developers endorse Parcelable over Serializable due to its improved speed.

The downside to Parcelable is that implementing it brings a ton of boilerplate. Here is a "small" example of a Parcelable class:

import android.os.{Parcel, Parcelable}

class Foo(val bar: String) extends Parcelable {
  override def describeContents: Int = 0

  override def writeToParcel(dest: Parcel, flags: Int): Unit = dest.writeString(bar)
}

object Foo {
  val CREATOR = new Parcelable.Creator[Foo] {
    override def createFromParcel(source: Parcel): Foo = new Foo(source.readString())

    override def newArray(size: Int): Array[Foo] = new Array(size)
  }
}

This seems like a task well-suited to Scala's implicits/traits. In particular, I was hoping for a solution that incorporates some of these ideas:

  1. Implicit conversions from Java's primitive types to Parcelable. Currently, Parcel has a bazillion different methods for reading/writing primitive types (e.g., readInt()/writeString(), readString()/writeString(), etc.) plus readParcelable()/writeParcelable(). I would like to only have to use the latter, so having implicit conversions from Int, String, etc. to Parcelable would be very useful.
  2. Parcelable conversions for commonly-used collections. For example, List[P], where P has an implicit conversion to Parcelable.
  3. Some sort of simpler trait to extend that allows a class to be Parcelable. (SParcelable?)

The biggest obstacle that prevents me from doing this myself is Parcelable's unusual requirement that all classes implementing it must have a static field named CREATOR (there exists a Scala compiler hack that allows you to define CREATOR as a value in the class's companion object). There would need to be some way of retrofitting classes such as Int and String to have a static CREATOR field, and a way for classes which extend SParcelable to have a static CREATOR field.

I'm not sure what the best way of approaching this is, so I'm open to suggestions. Scala macros are the only way that I can think of accomplishing this, but I'm not very well-versed in macros.

tvierling commented 10 years ago

Because of the way that the compiler hack works, the below is one of the few configurations I could make that works for the Parcelable interface itself. The main class has to extend Parcelable directly (extending a trait that itself extends Parcelable is not sufficient). I thought about using reflection to call a constructor taking Parcel as a parameter, but thought it would be better if the implementer chose whether to make it a constructor or to create a blank object and fill it in piecemeal.

import android.os._

abstract class ParcelableCompanion[T <: AnyRef](implicit tag: ClassTag[T]) {
  def readFromParcel(in: Parcel): T

  final val CREATOR = new Parcelable.Creator[T] {
    override def createFromParcel(in: Parcel): T = readFromParcel(in)

    override def newArray(size: Int): Array[T] = tag.newArray(size)
  }
}

You would use this as follows:

class Foo extends Parcelable {
  override def describeContents() = 0
  override def writeToParcel(p: Parcel, flags: Int) {}
}

object Foo extends ParcelableCompanion[Foo] {
  override def readFromParcel(p: Parcel) = new Foo
}

The main convenience you get here is not having to worry about the CREATOR field at all; that's taken care of for you (even the newArray() method). I'll comment on the conversion ideas separately.

tvierling commented 10 years ago

As for dealing with the Parcel itself, I don't know what you mean by instrumenting the base value types with a CREATOR field. Rather, it might be more useful to have an enrichment of Parcel that can be used with operators (such as += for writeToParcel) to make the methods look better visually.

The whole point of Parcelable is not to have the fields looked up via reflection; instead, you're writing the data in a compact format yourself, choosing how it gets encoded. There should not be any layer trying to make Parcelable work the same way that Serializable does: you don't necessarily want every single var in your class to be stuffed into the stream, and something has to take care of what happens when vars in the class change. Since Parcelable is used for inter-process communication, sometimes even between completely different apps, the format must be designed by hand for encoding and decoding.

RyanGlScott commented 10 years ago

That ParcelableCompanion class definitely seems more intuitive, and given the unusual constraints, is probably the neatest solution. (Also, thank you for showing me that trick with AnyRef and ClassTag. That solves a compilation problem I was having.)

That actually makes a simpler version of Parcelable really easy, now that I think about it:

trait SParcelable extends Parcelable {
  override def describeContents(): Int = 0
  override def writeToParcel(dest: Parcel, flags: Int): Unit
}

object SParcelable extends ParcelableCompanion[SParcelable] {
  override def readFromParcel(in: Parcel): SParcelable = in.readParcelable[SParcelable](SParcelable.getClass.getClassLoader)
}

That way, extending SParcelable only requires overriding one method.

RyanGlScott commented 10 years ago

As for your comments about Parcel, let me try to better explain what my goal is. Believe me, I'm not trying to turn Parcelable into a Serializable clone. Far from it—I want to carefully specify a way for collection data to be Parcelable. This is an example of what I'm thinking of:

object RichList {
  implicit class ParcelableList[T <: SParcelable](l: List[T]) extends SParcelable {
    override def writeToParcel(dest: Parcel, flags: Int): Unit = {
      dest.writeInt(l.size)
      l.foreach(dest.writeParcelable(_, flags))
    }
  }

  implicit object ParcelableList extends ParcelableCompanion[List[T]] {
    override def readFromParcel(in: Parcel): List[T] = {
      val size: Int = in.readInt()
      (1 to size).map(in.readParcelable(ParcelableList.getClass.getClassLoader)).toList
    }
  }
}

(This doesn't compile since I can't think of a good way of getting List to be parameterized correctly in an object, but you get the idea.)

A problem would arise if I wanted to parcel a List[Int], for example. Int is not Parcelable, so the above approach would not work.

tvierling commented 10 years ago

Note, the problem I found trying to do trait SParcelable extends Parcelable is that the Scala compiler hack for the CREATOR field won't kick in unless the concrete class also has extends Parcelable or with Parcelable in its type signature. Might be something that can be made more lenient in the compiler. (My first shot at it was precisely what you have there for SParcelable, though I didn't include writeToParcel and just let it inherit from the interface.)

You can't do the object SParcelable thing, though. The reason my example has a type parameter is because it is needed to make Parcelable.Creator<T>.newArray() work properly by returning a properly typed array (this isn't generic; the JVM actually does keep type information about the element type of an array). So in my example, Foo is intended to be the final, concrete class implementing Parcelable, not an intermediate layer.

tvierling commented 10 years ago

Oh, btw, if the compilation problem you were having was a typeness mismatch where you have T and the compiler wants T with Object, the <: AnyRef is the fix.

The ClassTag is used by the implementation of ParcelableCompanion solely to implement the typesafe array creation in newArray().

tvierling commented 10 years ago

To do what you want regarding parceling a List with arbitrary types which have their own, separate methods on Parcel, you're looking for something different. Like I said, maybe an enrichment of Parcel itself which uses overloaded methods (or an infix operator method) is better suited to this case. The bottom line is that basic types eventually have to call some other named method on Parcel to get encoded appropriately.

You'd also need to add some kind of type tag at the head of the encoded List so that your decoder knows what methods to use when reading it back in, and what the appropriate output type is. Alternatively, this could be inferred from an appropriately placed ClassTag on your decoder method.

RyanGlScott commented 10 years ago

How would overloaded methods in Parcel help? Isn't ParcelableList[T] only aware that T is an instance of Parcelable?

Also, what kind of issue were you having with trait SParcelable? The following code works for me on Scala 2.11.1:

import android.os.Parcel

class Bar(val baz: String) extends SParcelable {
  override def writeToParcel(dest: Parcel, flags: Int): Unit = dest.writeString(baz)
}

object Bar extends ParcelableCompanion[Bar] {
  override def readFromParcel(in: Parcel): Bar = new Bar(in.readString())
}

abstract class ParcelableCompanion[T <: AnyRef](implicit tag: ClassTag[T]) {
  def readFromParcel(in: Parcel): T

  final val CREATOR = new Parcelable.Creator[T] {
    override def createFromParcel(in: Parcel): T = readFromParcel(in)

    override def newArray(size: Int): Array[T] = tag.newArray(size)
  }
}

trait SParcelable extends Parcelable {
  override def describeContents(): Int = 0
}

object SParcelable extends ParcelableCompanion[SParcelable] {
  override def readFromParcel(in: Parcel): SParcelable = in.readParcelable[SParcelable](SParcelable.getClass.getClassLoader)
}
tvierling commented 10 years ago

Remove object SParcelable extends ParcelableCompanion[SParcelable]. There is no reason for it to exist, as there are no concrete classes of exactly the type SParcelable.

Build, and use javap path/to/class/Bar. See whether you have a method java.lang.Object CREATOR() or a field android.os.Parcelable$Creator CREATOR. In my testing with scala 2.11.1, the field only appears if Bar specifically says extends Parcelable or with Parcelable (in addition to any other classes/traits).

tvierling commented 10 years ago

As for your ParcelableList, well, it will work with anything that is Parcelable. The whole discussion around other methods on Parcel has to do with containers for things which are not Parcelable (e.g., Int or String; these are specifically supposed to be written and read with other methods on Parcel).

tvierling commented 10 years ago

Hm, and come to think of it, I should have declared

abstract class ParcelableCompanion[T <: Parcelable]

in the first place. It doesn't need <: AnyRef in that case, but it does properly guarantee typeness of what it's providing in readFromParcel().

tvierling commented 10 years ago

In fact, the presence of object SParcelable above is causing a problem: it creates

public interface SParcelable extends android.os.Parcelable {
  public static final android.os.Parcelable$Creator CREATOR;
  ...
}

which is a field that is the Parcelable.Creator<SParcelable> instance. This is very bad; you don't want that field to exist and be mistakenly pulled in by the parceling logic. It really must be only on the companion object for the concrete class, so remove object SParcelable.

tvierling commented 10 years ago

Building your example above yields this class file structure for Bar:

public class Bar implements SParcelable {
  public static Bar readFromParcel(android.os.Parcel);
  public static java.lang.Object CREATOR();
  public int describeContents();
  public java.lang.String baz();
  public void writeToParcel(android.os.Parcel, int);
  public Bar(java.lang.String);
}

Note the CREATOR() method, rather than Android's required CREATOR field. To fix this, you need to add with Parcelable as below:

class Bar(val baz: String) extends SParcelable with Parcelable {
  override def writeToParcel(dest: Parcel, flags: Int): Unit = dest.writeString(baz)
}

object Bar extends ParcelableCompanion[Bar] {
  override def readFromParcel(in: Parcel): Bar = new Bar(in.readString())
}

abstract class ParcelableCompanion[T <: Parcelable](implicit tag: ClassTag[T]) {
  def readFromParcel(in: Parcel): T

  final val CREATOR = new Parcelable.Creator[T] {
    override def createFromParcel(in: Parcel): T = readFromParcel(in)

    override def newArray(size: Int): Array[T] = tag.newArray(size)
  }
}

trait SParcelable extends Parcelable {
  override def describeContents(): Int = 0
}

This yields, instead:

public class Bar implements SParcelable {
  public static final android.os.Parcelable$Creator CREATOR;
  public static {};
  public int describeContents();
  public java.lang.String baz();
  public void writeToParcel(android.os.Parcel, int);
  public Bar(java.lang.String);
}

This is why Parcelable must be specifically given in Bar's list of extended traits. That's how the compiler knows to invoke the field hack.

And since at this point SParcelable does nothing but implement describeContents() for you, it's easier just to implement Parcelable directly, leaving only:

class Bar(val baz: String) extends Parcelable {
  override def writeToParcel(dest: Parcel, flags: Int): Unit = dest.writeString(baz)
  override def describeContents(): Int = 0
}

object Bar extends ParcelableCompanion[Bar] {
  override def readFromParcel(in: Parcel): Bar = new Bar(in.readString())
}

abstract class ParcelableCompanion[T <: Parcelable](implicit tag: ClassTag[T]) {
  def readFromParcel(in: Parcel): T

  final val CREATOR = new Parcelable.Creator[T] {
    override def createFromParcel(in: Parcel): T = readFromParcel(in)

    override def newArray(size: Int): Array[T] = tag.newArray(size)
  }
}

(This is why I didn't call it SParcelableCompanion, btw: I didn't see the point in creating a SParcelable at all.)

RyanGlScott commented 10 years ago

I compiled it with pfn's android-sdk-plugin and SBT 0.13.5, and it worked fine. I'm not sure what the best way to generate .class files with SBT is; otherwise, I'd show you the output of javap.

tvierling commented 10 years ago

By "work" you do mean that you parceled and unparceled a Bar value using Parcel.{write,read}Parcelable (not any methods in Bar), correct? Have you also tried parceling and unparceling an Array[Bar] using Parcel.{write,read}ParcelableArray() yet? (This will exercise the newArray() method on read.)

Class files from SBT normally end up in target/scala-2.X/classes or subproject/target/scala-2.X/classes as part of the build. They have to exist before dexing is done, so they're probably still in your build tree (or maybe a jar containing them is). You could try find . -name Bar.class.

However, not all the world's a sbt -- many will soon be using IDEA, for instance, as it's the basis for Android Studio -- so it's important for a solution to work for everyone. AFAICT, the 2.11.1 compiler will only generate the field required by Android to unparcel if the concrete class specifically has Parcelable in its first-order list of extends.

RyanGlScott commented 10 years ago

You're right, the results of javap Bar.class (in my bin folder, as it turns out) reveal that a CREATOR() method is generated, not a CREATOR field. Now I'm amazed that this code is even running at all. Regardless, I'll just suck it up and override describeContents each time.

I'm still struggling to think of a good way to make List[T] an instance of Parcelable without manually defining instances for each primitive type. I suppose some sort of code generation (which Scaloid uses, apparently) could achieve this effect, but I can't help but feel like there's a better way of achieving the same end.

tvierling commented 10 years ago

This is only a partially formed thought, but this should illustrate where my mind was going with regard to type conversions. I was contemplating either a RichParcel with overloaded methods by type, or implicit conversions to a specialized type (not actually Parcelable). A half-implementation of the latter follows -- it's the writing path only.

The colon-tail operator +=: makes it a more visual syntax, since it is actually implemented on a type converter that is right-bound. The alternative, a RichParcel, would use a left-bound method instead. The reason the right-binding form below appeals more to me is because of the view bound on the Iterable[T] method: it ensures at compile time that the member type of the collection can be parceled (via implicit conversion to CanParcel).

import android.os._
import scala.language.implicitConversions

trait CanParcel {
  def +=:(p: Parcel): Unit
}

object CanParcel {
  implicit def canParcel(obj: Parcelable) = new CanParcel {
    override def +=:(p: Parcel) { p.writeParcelable(obj, 0) }
  }

  implicit def canParcel(i: Int) = new CanParcel {
    override def +=:(p: Parcel) { p.writeInt(i) }
  }

  implicit def canParcel[T <% CanParcel](obj: Iterable[T]) = new CanParcel {
    override def +=:(p: Parcel) {
      p.writeInt(obj.size)
      obj.foreach(p +=: _)
    }
  }
}

The reading path needs to work a different way; it may require a large single method with a pattern match on a ClassTag (the compiler generates the appropriate ClassTag, and then the pattern match figures out which method(s) to call to generate the appropriate output data).

tvierling commented 10 years ago

And to reduce the amount of object churn, it can make use of universal traits and value classes:

import android.os._
import scala.language.implicitConversions

trait CanParcel extends Any {
  def +=:(p: Parcel): Unit
}

object CanParcel {
  implicit class CanParcelParcelable(private val obj: Parcelable)
  extends AnyVal with CanParcel {
    def +=:(p: Parcel) { p.writeParcelable(obj, 0) }
  }

  implicit class CanParcelInt(private val i: Int)
  extends AnyVal with CanParcel {
    def +=:(p: Parcel) { p.writeInt(i) }
  }

  implicit def canParcel[T <% CanParcel](obj: Iterable[T]) = new CanParcel {
    def +=:(p: Parcel) {
      p.writeInt(obj.size)
      obj.foreach(p +=: _)
    }
  }
}

(The Iterable[T] has to remain a conventional implicit conversion because of the extra parameter introduced by the view bound.)

RyanGlScott commented 10 years ago

I really like that idea. I'm away from my computer for a week; otherwise, I'd try it out and see how it works. I'd love to see something like this included with Scaloid.

RyanGlScott commented 10 years ago

Alright, I've had the chance to look at it. I feel like a good solution is close, but there are some lingering issues:

RyanGlScott commented 10 years ago

Actually, upon further thought, it would probably be wiser to have a companion trait to ToParcel (which I'll hastily call FromParcel) that diverts around the need for a class to implement Parcelable. Here's a quick mockup:

package com.rscott.scaloid.utils

import android.os._
import scala.language.implicitConversions

trait ToParcel extends Any {
  def +=:(p: Parcel): Unit
}

trait FromParcel[A] extends Any {
  def read(p: Parcel, cl: ClassLoader): A
}

class RichParcel(val p: Parcel)(implicit cl: ClassLoader) {
  def read[A](implicit ev: FromParcel[A]): A = ev.read(p, cl)
}

object RichParcel {
  implicit val classLoader: ClassLoader = RichParcel.getClass.getClassLoader

  implicit def enrichParcel(p: Parcel)(implicit cl: ClassLoader): RichParcel = new RichParcel(p)(cl)

  implicit class ToParcelParcelable(private val obj: Parcelable) extends AnyVal with ToParcel {
    override def +=:(p: Parcel): Unit = p.writeParcelable(obj, 0)
  }

  implicit def fromParcelParcelable[Parcelable] = new FromParcel[Parcelable] {
    override def read(p: Parcel, cl: ClassLoader): Parcelable = p.readParcelable(cl)
  }

  implicit class ToParcelInt(private val i: Int) extends AnyVal with ToParcel {
    override def +=:(p: Parcel): Unit = p.writeInt(i)
  }

  implicit def fromParcelInt = new FromParcel[Int] {
    override def read(p: Parcel, cl: ClassLoader): Int = p.readInt()
  }

  implicit def toParcelIterable[T](obj: Iterable[T])(implicit ev: T => ToParcel) = new ToParcel {
    override def +=:(p: Parcel): Unit = {
      p.writeInt(obj.size)
      obj.foreach(p +=: ev(_))
    }
  }

  implicit def fromParcelList[T](implicit ev: T => FromParcel[T]) = new FromParcel[List[T]] {
    override def read(p: Parcel, cl: ClassLoader): List[T] = {
      val size: Int = p.readInt()
      (1 to size).map(_ => p.read[T]).toList
    }
  }
}

And an example of using it:

import android.os.{Parcel, Parcelable}
import com.rscott.scaloid.utils.RichParcel._

class Bar(val baz: Int, val quux: List[Int]) extends Parcelable {
  override def describeContents: Int = 0
  override def writeToParcel(dest: Parcel, flags: Int): Unit = {
    dest +=: baz
    dest +=: quux
  }
}

object Bar extends ParcelableCompanion[Bar] {
  override def readFromParcel(in: Parcel): Bar = new Bar(in.read[Int], in.read[List[Int]])
}

I've tested this with a simple example, and it seems to work great. That leaves two pieces left that I'd want:

pocorall commented 10 years ago

I had not participated in this thread because I don't recommend to use Parcelable at all. It has the worst design it can be. There are several elegant alternatives:

Java serialization

Why don't use Java serialization? Modern Android devices are fast enough to serialize moderate sized objects.

Do not serialize

If your object is big, and serialization time matters, do not serialize it. Pass your object via a static value or LocalService.

Other serializers

Although I had not tested in Android devices, there are some other serializers including https://github.com/scala/pickling and https://github.com/EsotericSoftware/kryo .

Write your own

Definately this would be far better than Parcelable.

RyanGlScott commented 10 years ago

Believe me, I hate Parcelable too. Unfortunately, I have to keep coming back to it primarily because I need to serialize a fairly large ArrayList (which I should probably convert to an ArrayBuffer if I'm switching to Scala) across device rotations, which is when you can really notice how slow Serializable is on Android.

However, I hadn't heard of the pickling library before. That actually might solve most of my problems for me, since it provides a nice, typeclassy, and (allegedly) fast way of serializing complex data structures into primitive types, and providing an implicit conversion to a Pickle subclass is probably nicer than extending Parcelable. I'll close this issue before I try that out.

pocorall commented 10 years ago

Let me know whether the pickling works fine in Android or not. :smile:

RyanGlScott commented 10 years ago

Unfortunately, pickling can't be used on Android, and it looks like Scala's reflection library is to blame. Currently, calling pickle invokes a method which reflectively uses java.rmi.Remote, but the java.rmi package is not included with the Android SDK.

pocorall commented 10 years ago

It also a great news that pickling team will fix this issue.