Handling ImmutableList in Data.clone

Created to fix http://code.google.com/p/google-http-java-client/issues/detail?id=71 : Data.clone cannot handle ImmutableList.

[yanivi, 2012-06-21 00:58:08]

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
File google-http-client/src/main/java/com/google/api/client/util/Data.java
(right):

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/util/Data.java:232: }
else if (data instanceof ImmutableList<?>) {
I am worried about how much larger this makes a ProGuarded Android application,
since the whole com.google.collect package is huge.  After all, ProGuard can
only strip out what you don't use.  Once you use it, ProGuard can't really help
you.

I'm not saying we shouldn't necessarily do it.  Just that we should measure its
impact.

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/util/Data.java:233:
Collection<Object> destCollection = new ArrayList<Object>();
why the intermediate variable?  why note just add each item to the
ImmutableList.Builder directly?

[rmistry, 2012-06-21 12:13:37]

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
File google-http-client/src/main/java/com/google/api/client/util/Data.java
(right):

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/util/Data.java:232: }
else if (data instanceof ImmutableList<?>) {
On 2012/06/21 00:58:08, yanivi wrote:
> I am worried about how much larger this makes a ProGuarded Android
application,
> since the whole com.google.collect package is huge.  After all, ProGuard can
> only strip out what you don't use.  Once you use it, ProGuard can't really
help
> you.
> 
> I'm not saying we shouldn't necessarily do it.  Just that we should measure
its
> impact.

I understand your concern but maybe we should not worry about this because there
was a need for this change and it is likely we will need to use
com.google.common.collect in more places.

http://codereview.appspot.com/6308097/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/util/Data.java:233:
Collection<Object> destCollection = new ArrayList<Object>();
On 2012/06/21 00:58:08, yanivi wrote:
> why the intermediate variable?  why note just add each item to the
> ImmutableList.Builder directly?

Done.

[yanivi, 2012-07-30 21:14:20]

http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
File google-http-client/src/main/java/com/google/api/client/util/Data.java
(right):

http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/util/Data.java:232: }
else if (data instanceof ImmutableList<?>) {
Some concrete numbers: the installed size of calendar-preview-android-sample
goes from 328KB to 356KB, which is 28KB more.  I'm still undecided about this
change.

[rmistry, 2012-07-31 15:30:51]

http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
File google-http-client/src/main/java/com/google/api/client/util/Data.java
(right):

http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/util/Data.java:232: }
else if (data instanceof ImmutableList<?>) {
On 2012/07/30 21:14:20, yanivi wrote:
> Some concrete numbers: the installed size of calendar-preview-android-sample
> goes from 328KB to 356KB, which is 28KB more.  I'm still undecided about this
> change.

12.7% increase. Your call on whether we should do this or defer it till we
absolutely need a com.google.collect dependency.

[yanivi, 2012-07-31 16:53:52]

On 2012/07/31 15:30:51, rmistry wrote:
>
http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
> File google-http-client/src/main/java/com/google/api/client/util/Data.java
> (right):
> 
>
http://codereview.appspot.com/6308097/diff/5001/google-http-client/src/main/j...
> google-http-client/src/main/java/com/google/api/client/util/Data.java:232: }
> else if (data instanceof ImmutableList<?>) {
> On 2012/07/30 21:14:20, yanivi wrote:
> > Some concrete numbers: the installed size of calendar-preview-android-sample
> > goes from 328KB to 356KB, which is 28KB more.  I'm still undecided about
this
> > change.
> 
> 12.7% increase. Your call on whether we should do this or defer it till we
> absolutely need a com.google.collect dependency.

Defer.  Of course keep the diff in case we decide to do it later.