[cwac-endless and cwac-thumbnail] feedback & question

[MarcoAndroid]

Hi Mark,

I've been trying to combine the use of cwac-endless (cwace) and cwac-
thumbnail (cwact). Here's some feedback & question about that.

In my own code I was using SoftReferences, but like as mentioned <a
href="http://blog.jteam.nl/2009/09/17/exploring-the-world-of-android-
part-2/">here</a> by you: it does work, but SoftReferences seem to get
garbage collected really fast (too fast?), so scrolling up & down fast
still becomes notably "choppy"; not in an unusable way, but still ...
it could be better. Thus I went on a quest for the ultimate solution,
and that's how I ended up here :)
Immediately one question pops up: it seems you don't use SoftReference
anymore in your code (or I can't search :). The nice thing about
SoftReference is that it should prevent an OutOfMemory exception
because it cleans up before that should happen (see <a href="http://
developer.android.com/reference/java/lang/ref/
SoftReference.html">here</a>).
How safe is your solution regarding OOM exceptions? I see you use a
maxSize with LRU algorithm, but those don't prevent an OOM.

So I tried the ThumbnailDemo first. That behaved quite strange: when
you start it for the very first time, you have to enter your twitter
username + password. But there are no buttons or whatsoever! So I just
waited but nothing happened, the username+password screen just stayed
on! Hitting the back button didn't do anything either (or it seemed).
Then after a while it did seem to do something & the username+password
screen disappeared & I saw a fine smooth scrolling list of tweets. It
would be nice if the Demo would be a bit more intuitive, like with a
"Login" button or something & then automatically show/go back to the
tweets page.

So then it was time for me to integrate cwace and cwact into my own
not-so-smooth-scrolling ListView + Adapter. Note that I already
implemented the optimization tips like using convertView &
ViewWrapper.
First I put in cwace, pretty smooth. Tricky part for me was that I
still wanted to show some text when nothing is found by the background
task. In rebindPendingView() I was replacing the progress-image part
via View.VISIBILE), but still the text wouldn't show! I figured out
that the reason for that is because the listadapter is empty (nothing
added in appendInBackground()). So now I manually add a dummy item to
that listadapter such that at least 1 entry is in there; and indeed
after doing that the "nothing found" text will be shown.
Maybe for a next version you can add a View as return to
rebindPendingView() indicating that that one should be shown in any
case? Hmm, not sure if that is the cleanest solution...

I constructed the adapters like this in my ListActivity:
List<MyData> myData = new ArrayList<MyData>();
MyAdapter myAdapter = new MyAdapter( myData, ... );
MyAdapterEndless myAdapterEndless = new MyAdapterEndless
( myAdapter, ... );
setListAdapter( myAdapterEndless );



Next: adding cwact to the above was quite tricky. I first tried this
in my ListActivity:
List<MyData> myData = new ArrayList<MyData>();
MyAdapter myAdapter = new MyAdapter( myData, ... );
MyAdapterEndless myAdapterEndless = new MyAdapterEndless
( myAdapter, ... );
ThumbnailAdapter thumbnailAdapter = new ThumbnailAdapter( this,
myAdapterEndless, cache, imageIDs );
setListAdapter( myAdapterEndless );

That *almost* immediately worked. What didn't work:
- The image in the first entry (the pendingView) was never put in. I
found out why: in rebindPendingView() I replaced all Views correctly,
but ThumbnailAdapter.getView() never gets called anymore for that
(pendingview) position. Makes sense: ThumbnailAdapter doesn't know
anything about EndlessAdapter's "internal" updates like
rebindPendingView. Am I right?
- One time when I was scrolling down I got a nullpointer exception in
rebindPendingView(). The 'convertView' parameter was null! Couldn't
reproduce this one anymore, but quite odd I'd say. Any idea how this
could have happened? How could the pending-view have turned null in
the mean time?

In the end what did work (after a lot of trial and error):
List<MyData> myData = new ArrayList<MyData>();
MyAdapter myAdapter = new MyAdapter( myData, ... );
ThumbnailAdapter thumbnailAdapter = new ThumbnailAdapter( this,
myAdapter, cache, imageIDs );
MyAdapterEndless myAdapterEndless = new MyAdapterEndless
( myAdapter, ... );
setListAdapter( myAdapterEndless );
(note that I swapped thumbnailadapter and endlessadapter)

Additionally, in rebindPendingView() I added an explicit call to
thumbnailAdapter.getView(), so it would try to get the image for the
replaced entry too.

My big question is: did I do it the right way in the end, or is there
a better way? I've got the feeling it can be done cleaner, but I can
also see that the cwace's-pendingView-trick can confuse wrapping
adapters...

Oh, and a few other feedback things:
- Why didn't you make the ThumbnailDemo more like the other demos,
where the demo class extends the adapter being demoed?
- In ThumbnailAdapter.java, the comment for getView() seems a copy &
paste from EndlessAdapter?
- I see you log errors via Log.e(...). It would be handy to provide a
flag one can set to turn on/off this kind of logging (based on <a
href="http://groups.google.com/group/android-developers/browse_thread/
thread/afc9ef79afa4ec4b?pli=1">this recommendation</a>)
- It would be great if you could put a picture on your site how cwact
works together with all the other packages you're providing.



Regards,
Marco

[Mark Murphy]

MarcoAndroid wrote:
> Immediately one question pops up: it seems you don't use SoftReference
> anymore in your code (or I can't search :). The nice thing about
> SoftReference is that it should prevent an OutOfMemory exception
> because it cleans up before that should happen (see <a href="http://
> developer.android.com/reference/java/lang/ref/
> SoftReference.html">here</a>).

Except that it doesn't work on Android, at least in the 1.5 timeframe. I
have no idea if the GC SoftReference bugs have been fixed.
SoftReferences get GC'd too soon with this bug.

> I see you use a
> maxSize with LRU algorithm, but those don't prevent an OOM.

Correct. If you experience OOM with your thumbnails, choose a smaller
cache size.

> - The image in the first entry (the pendingView) was never put in. I
> found out why: in rebindPendingView() I replaced all Views correctly,
> but ThumbnailAdapter.getView() never gets called anymore for that
> (pendingview) position. Makes sense: ThumbnailAdapter doesn't know
> anything about EndlessAdapter's "internal" updates like
> rebindPendingView. Am I right?

That depends on how you construct your pending view. Have your thumbnail
ImageView be in the pending view, just invisible/gone, and
ThumbnailAdapter will work fine.

> In the end what did work (after a lot of trial and error):
> List<MyData> myData = new ArrayList<MyData>();
> MyAdapter myAdapter = new MyAdapter( myData, ... );
> ThumbnailAdapter thumbnailAdapter = new ThumbnailAdapter( this,
> myAdapter, cache, imageIDs );
> MyAdapterEndless myAdapterEndless = new MyAdapterEndless
> ( myAdapter, ... );
> setListAdapter( myAdapterEndless );
> (note that I swapped thumbnailadapter and endlessadapter)

ThumbnailAdapter wraps EndlessAdapter which wraps the underlying data,
in the 2-3 places I have used the combination of the two.

> - In ThumbnailAdapter.java, the comment for getView() seems a copy &
> paste from EndlessAdapter?

Yes, that's a flaw. I will look to correct this and some of the other
things you have pointed out, when I elect to more formally document
these adapters, probably in conjunction with a chapter in _The Busy
Coder's Guide to *Advanced* Android Development_.

Thanks for your feedback!

--
Mark Murphy (a Commons Guy)
http://commonsware.com | http://twitter.com/commonsguy

Android App Developer Training: http://commonsware.com/training

[MarcoAndroid]

Thanks for the quick reply! I quickly managed to mock up a version of
EndlessAdapterDemo reproducing the problem that "no data found" is not
showing if there are no items in the adapter:

Below is the code of the EndlessAdapterDemo, with some changes:
- The DemoAdapter starts with 0 entries (see onCreate()) instead of
some initial items
- appendInBackground() doesn't add anything to the adapter. It
simulates finding no data (this is possible in the very first call,
like a search to a webservice that doesn't return anything).
- rebindPendingView() had to be modified such that it checks for 0
entries in the adapter, otherwise you get IndexOutOfBoundsException

The modified code for the EndlessAdapterDemo:

public class EndlessAdapterDemo extends ListActivity {
private static String[] items={"lorem", "ipsum", "dolor",
"sit", "amet", "consectetuer",
"adipiscing", "elit", "morbi",
"vel", "ligula", "vitae",
"arcu", "aliquet", "mollis",
"etiam", "vel", "erat",
"placerat", "ante",
"porttitor", "sodales",
"pellentesque", "augue",
"purus"};

@Override
public void onCreate(Bundle icicle) {
super.onCreate(icicle);
setContentView(R.layout.main);

// Original code:
// setListAdapter(new DemoAdapter(new ArrayList<String>(Arrays
// .asList(items))));

// Start with an empty list, so pendingView will be called
immediately
setListAdapter(new DemoAdapter(new ArrayList<String>()));
}

class DemoAdapter extends EndlessAdapter {
private RotateAnimation rotate = null;

DemoAdapter(ArrayList<String> list) {
super(new ArrayAdapter<String>(EndlessAdapterDemo.this,
R.layout.row, android.R.id.text1, list));

rotate = new RotateAnimation(0f, 360f, Animation.RELATIVE_TO_SELF,
0.5f, Animation.RELATIVE_TO_SELF, 0.5f);
rotate.setDuration(600);
rotate.setRepeatMode(Animation.RESTART);
rotate.setRepeatCount(Animation.INFINITE);
}

protected View getPendingView(ViewGroup parent) {
View row = getLayoutInflater().inflate(R.layout.row, null);

View child = row.findViewById(android.R.id.text1);

child.setVisibility(View.GONE);

child = row.findViewById(R.id.throbber);
child.setVisibility(View.VISIBLE);
child.startAnimation(rotate);

return (row);
}

protected void rebindPendingView(int position, View row) {

View child = row.findViewById(android.R.id.text1);

child.setVisibility(View.VISIBLE);

// Added this 'if' statement because with the appendInBackground()
changes you'd otherwise
// get an IndexOutofBoundsException: invalid location 0, size = 0.
if (getWrappedAdapter().getCount() <= 0) {
((TextView) child).setText("No data found");
// Enable below 2 rows to *do* see the "No data found" text appear
// ArrayAdapter<String> a = (ArrayAdapter<String>)
getWrappedAdapter();
// a.add("dummy");
} else {
((TextView) child).setText(getWrappedAdapter().getItem(position)
.toString());
}

child = row.findViewById(R.id.throbber);
child.setVisibility(View.GONE);
child.clearAnimation();
}

@SuppressWarnings("unchecked")
protected boolean appendInBackground() {

Log.d(getClass().getSimpleName(), "going to sleep");

SystemClock.sleep(2000); // pretend to do work

ArrayAdapter<String> a = (ArrayAdapter<String>) getWrappedAdapter
();

// Original code:
// for (String item : items) {
// a.add(item);
// }
//
// return (a.getCount() < 80);

// Don't add anything to the list.
// This simulates no data found (this can happen on the very first
remote call for example)

// Of course no more data after that thus return false
return (false);
}
}
}



If you run it you'll see the rotator spin for 2 seconds and then
disappear. But the "No data found" text is not shown! My guess is thus
that occurs because the list is of size 0...
Now if you add this below (TextView) child).setText("No data found")
and rerun the app, you *will* see the "No data found" text (as
expected)!
ArrayAdapter<String> a = (ArrayAdapter<String>) getWrappedAdapter();
a.add("dummy");

Hope this helps.

Hmm, must be something in my code then. Will double check if I do
something wrong in the visible/gone part of replacePendingView()...

>
> > - In ThumbnailAdapter.java, the comment for getView() seems a copy &
> > paste from EndlessAdapter?
>
> Yes, that's a flaw. I will look to correct this and some of the other
> things you have pointed out, when I elect to more formally document
> these adapters, probably in conjunction with a chapter in _The Busy
> Coder's Guide to *Advanced* Android Development_.
>
> Thanks for your feedback!
>
> --

> Mark Murphy (a Commons Guy)http://commonsware.com|http://twitter.com/commonsguy

[Mark Murphy]

MarcoAndroid wrote:
> Thanks for the quick reply! I quickly managed to mock up a version of
> EndlessAdapterDemo reproducing the problem that "no data found" is not
> showing if there are no items in the adapter:

Ah. I see where things are going sideways.

Here are some rules of thumb with the Thumbnail/Endless combo:

-- Don't set the list adapter until you have data. After all, there's
nothing to show, anyway, so there's no point in setting an empty adapter
on a list. Just use a progress indicator of some kind until you have
some data.

-- If, after you fetch the initial round of data, you know you have all
of the results already (e.g., Web service API indicates you received
search results page 1 of 1), don't bother with the EndlessAdapter, and
just use the ThumbnailAdapter.

-- In the "normal" case, where once you have data you know there will be
more, use the combination.

If you use those rules of thumb, you should avoid the problem. I may try
to make this stuff a bit more robust sometime, though using the above
rules of thumb should also help performance and memory consumption, so
they're a good idea anyway.

--
Mark Murphy (a Commons Guy)
http://commonsware.com | http://twitter.com/commonsguy

Android App Developer Books: http://commonsware.com/books

[MarcoAndroid]

Okay, I'll keep that in mind!

I have one more question related to cwace and cwact: Should the
getCache() call be synchronized? In Application.java in the Cache demo
it is, in Application.java in the Thumbnail demo it is not.

Regards,
Marco

[Mark Murphy]

In the Cache demo, I lazy-create the cache, and so I synchronize the
getter to ensure I don't create two caches on parallel threads
simultaneously. In the Thumbnail demo, I create the cache at Application
instantiation time, and so such thread contention is not needed. The cache
itself should be thread-safe IIRC.

--
Mark Murphy (a Commons Guy)
http://commonsware.com

Android App Developer Books: http://commonsware.com/books.html