2

I'm developing an Android app in which I've got some lists. It's more complex than I'll explain here, but the scenario I'm posting behaves unexpectedly as well.
I've been browsing several questions in Stack Overflow but none that I found solves this issue.

So, for the sake of the question I've built this scenario. I've got an Activity with an array of Strings, like so:

public class TestViewlazyloadActivity extends Activity {
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        String[] items = { "lorem", "ipsum", "dolor", "sit", "amet" };

        ListView list = (ListView) findViewById(R.id.testListView);
        TestAdapter adapter = new TestAdapter(this, R.layout.list_item, items);

        list.setAdapter(adapter);
    }
}

Being my two layouts this way:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="fill_parent"
    android:layout_height="fill_parent"
       android:orientation="vertical" >

    <ListView
        android:id="@+id/testListView"
        android:layout_width="fill_parent"
        android:layout_height="wrap_content" >
    </ListView>

</LinearLayout>

And this way:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="fill_parent"
    android:layout_height="fill_parent"
    android:orientation="vertical" >

    <TextView
        android:id="@+id/testTextView"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:textAppearance="?android:attr/textAppearanceLarge" />

</LinearLayout>

Pretty simple. Then I've implemented an ArrayAdapter class with the ViewHolder idea, which getView() method is as follows:

public View getView(int position, View convertView, ViewGroup parent) {
    ViewHolder holder = null;
    String item = this.getItem(position);

    if(convertView == null) {
        LayoutInflater inflater = ((Activity) this.getContext()).getLayoutInflater();
        convertView = inflater.inflate(R.layout.list_item, null);

        holder = new ViewHolder();
        holder.textView = (TextView) convertView.findViewById(R.id.testTextView);
        convertView.setTag(holder);
    } else {
        holder = (ViewHolder) convertView.getTag();
    }

    final ViewHolder auxHolder = holder;
    LoadingService.capitalizeItemWithDelay(item, new LoadedCallback() {
        public void onItemLoaded(String item) {
            auxHolder.textView.setText(item);
        }
    });

    return convertView;
}

Being LoadedCallback an interface with just onItemLoaded. And LoadingService.capitalizeItemWithDelay() just starts a Thread which sleeps for a random amount of time (to simulate a lazy load) and calls this callback with the upper case String.

Normal behavior should be that user is viewing the list with 5 blank rows. After some time items begin to spawn in their respective rows, even in a random order given the random sleep timer.

What really happens is that items get loaded in the first row, one by one, and eventually some of them get set in their real place. It's like the auxHolder keeps the reference in its TextView to the first element, which is weird. I understand it's either because of the view being recycled or because of the auxHolder being final.

Any ideas? Thanks in advance.

Meta
  • 346
  • 2
  • 12

2 Answers2

1

Get rid of final on your ViewHolder variable and move ViewHolder auxHolder into the inner class.

Try this:

   ... 
    LoadingService.capitalizeItemWithDelay(item, new LoadedCallback() {
        ViewHolder auxHolder = holder;
        public void onItemLoaded(String item) {
            auxHolder.textView.setText(item);

When you use the final key word with a local variable, like auxHolder, you are saying that the value you give it, is the last value it will have. This is why you are having problems. Here is a link that will explain a little better: http://www.javapractices.com/topic/TopicAction.do?Id=23

BlackHatSamurai
  • 23,275
  • 22
  • 95
  • 156
  • Can't do that, because compiler will tell me `Cannot refer to a non-final variable auxHolder inside an inner class defined in a different method`. That's why that `final` modifier is there. – Meta Aug 14 '12 at 13:11
  • Did you define `auxHolder` in another method? – BlackHatSamurai Aug 14 '12 at 13:14
  • Nope, I didn't. That's all the code that goes around except the declaration of the ViewHolder class and the class which simulates the lazy loading (which has nothing to do with ViewHolders) – Meta Aug 14 '12 at 13:16
  • See my edit to the original code. Try moving the `auxHolder` definition into the inner method. – BlackHatSamurai Aug 14 '12 at 13:17
  • That doesn't work, as we would expect. Remember variables can't be used inside anonymous classes unless they are set to final. – Meta Aug 14 '12 at 13:20
  • That is not true. See this post: http://stackoverflow.com/questions/1299837/cannot-refer-to-a-non-final-variable-inside-an-inner-class-defined-in-a-differen You can use non `final` variables if they are local to the inner class. So move the variable definition inside the inner class and you should be fine :) – BlackHatSamurai Aug 14 '12 at 13:24
  • Inner classes can't use `static` variables, unless they are `final`. – BlackHatSamurai Aug 14 '12 at 13:30
  • I'm afraid my comment wasn't understood. Of course you can use non-final variables inside an anonymous class, but you can't refer to variables in the "outer" lass unless these variables are set to final. So I can't move the variable definition inside the inner class because the `holder` variable can't be referred unless it's final. That's why it that modifier is in that place :) – Meta Aug 14 '12 at 14:03
0

You may not be updating on the UI Thread. I would try performing your loading actions in a post and see that fixes everything. Try doing something like this

convertView.post(new Runnable() {
    @Override
    public void run() {
        LoadingService.capitalizeItemWithDelay(item, new LoadedCallback() {
            ViewHolder auxHolder = holder;
            public void onItemLoaded(String item) {
                auxHolder.textView.setText(item);
            }
        }
   }
});
Frank Sposaro
  • 8,511
  • 4
  • 43
  • 64
  • I'd need to set `item` variable as final to be able to use it inside the `run` method, and holder variable as `final` to be able to use them inside `LoadedCallback` definition – Meta Aug 14 '12 at 14:04