Some Parcel array storage enlightenment

If some of you have used SwissKnife, you may have also used the @Parcelable annotation. This annotation turns a normal class into a Parcelable object by writing all the boilerplate for you with an AST Transform - Groovy's way of Annotation Processing.

I was using Groovy on Android and SwissKnife on a recent project when I found a bug on the @Parcelable annotation. It couldn't restore an String[] from a Parcel item!

You see, to save and load a variable from a Parcel object, SwissKnife checks the type of the field and chooses the best method for the task. Storing a double? writeDouble(...) it is. Does it implement the Parcelable interface? Then it's a writeParcelable(...). But there was some kind of problem with the readStringArray(...) method.

So, I proceed to see my decompiled code - thanks IDEA 14 for that - and I saw it was doing it this way:

String[] slidePhotos = ...;

public MyClass(Parcel parcel) {
	...
    parcel.readStringArray(this.slidePhotos);
    ...
}

Which should be fine, shouldn't it? I mean, look at the Android docs!

Well, it seems like there was some kind of problem as this log error was shown:

java.lang.RuntimeException: bad array lengths

Bad array lengths? Ok, that doesn't look good. Let's take a closer look at Parcel's implementation:

public final void readStringArray(String[] val) {
  int N = readInt();
  if (N == val.length) {
    for (int i=0; i<N; i++) {
      val[i] = readString();
    }
  } else {
 	 throw new RuntimeException("bad array lengths");
  }
}

As you can see:

  1. First it reads the length of the array.
  2. It checks that the passed String[] parameter has the same length as the stored one.
  3. Then it loads every String, one at a time.

But this means you have to know the array size before you start reading! That doesn't sound logical, since to do that you'd have to read the stored length, which readStringArray(...) already does and would make it crash since that value wouldn't be there to read again. You could of course store it twice, but does it look like a good solution to you?

So I kept looking at the implementation and docs and found this other method called createStringArray(). Let's look at its implementation:

public final String[] createStringArray() {
	int N = readInt();
	if (N >= 0) {
		String[] val = new String[N];
		for (int i=0; i<N; i++) {
			val[i] = readString();
		}
		return val;
	} else {
		return null;
	}
}

Now this looks better! As you can see, this:

  1. Reads the length of the array.
  2. Then creates a new String[] with that legnth.
  3. It reads every String value and adds it to the array.
  4. Returns the String[] object or null if the read length was 0 or less.

This is what readStringArray(...) should be doing! So why is it even there when we have a proper replacement? And why the hell doesn't createStringArray() have proper documentation?

Well, that's Android's magic. You have tons of overdocumented stuff and some of the really interesting stuff totally blank.

So, for all of you that use SwissKnife, I'll be committing a bugfix for this ASAP. In the meantime, please use a List<String> variable instead.