Android ListView adapter loads wrong

Question

For some reason when I use this code it doesn't work properly. Instead of putting each crossroad in its place, it puts the last one in the first and the rest are empty.

package com.Itay.lightalaram.lightalarm;

import android.app.Activity;
import android.os.Bundle;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ArrayAdapter;
import android.widget.ListView;
import android.widget.TextView;

import java.util.ArrayList;
import java.util.List;

public class CrossroadsActivity extends Activity {
    private List<Crossroad> crossroads;
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.crossroads);

        crossroads = new ArrayList<Crossroad>();

        populateListView();
    }

    private void populateListView() {
        ListView list = (ListView) findViewById(R.id.crossroadsListView);
        crossroads.add(new Crossroad("Oranim", 50, 30));
        crossroads.add(new Crossroad("Evelina", 40, 85));
        crossroads.add(new Crossroad("Sacker", 43, 22));
        crossroads.add(new Crossroad("bagger", 43, 22));


        ArrayAdapter<Crossroad> adapter = new CrossroadAdapter();

        list.setAdapter(adapter);
    }

    private class CrossroadAdapter extends ArrayAdapter<Crossroad> {
        public CrossroadAdapter(){
            super(CrossroadsActivity.this, R.layout.item_crossroad, crossroads);
        }

        @Override
        public View getView(int position, View convertView, ViewGroup parent) {
            View itemView = convertView;
            if(null == itemView) {
                itemView = getLayoutInflater().inflate(R.layout.item_crossroad, parent, false);

                Crossroad currentCrossroad = crossroads.get(position);

                TextView name = (TextView) findViewById(R.id.item_crossroadName);
                TextView redLight = (TextView) findViewById(R.id.item_redLightDuration);
                TextView greenLight = (TextView) findViewById(R.id.item_greenLightDuration);
                try {
                    name.setText(currentCrossroad.getName());
                    redLight.setText("" + currentCrossroad.getRedLight());
                    greenLight.setText("" + currentCrossroad.getGreenLight());
                } catch (Exception e) {
                }
                return itemView;
            }
            return null;
        }
    }
}

Output:

I updated my answer with a `ViewHolder` pattern. You should take a look. Happy coding. — Blo, Apr 18 '14 at 11:49

Shini · Answer 1 · 2014-04-18T11:43:26.647

 private class CrossroadAdapter extends ArrayAdapter<Crossroad> {
    public CrossroadAdapter() {
        super(MainActivity.this, R.layout.item_crossroad, crossroads);
    }

    TextView name, redLight, greenLight;

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View itemView = convertView;
        Crossroad currentCrossroad = crossroads.get(position);
        if (null == itemView) {
            itemView = getLayoutInflater().inflate(R.layout.item_crossroad,
                    parent, false);

            name = (TextView) itemView
                    .findViewById(R.id.item_crossroadName);
            redLight = (TextView) itemView
                    .findViewById(R.id.item_redLightDuration);
            greenLight = (TextView) itemView
                    .findViewById(R.id.item_greenLightDuration);

        }
        try {
            name.setText(currentCrossroad.getName());
            redLight.setText("" + currentCrossroad.getRedLight());
            greenLight.setText("" + currentCrossroad.getGreenLight());
        } catch (Exception e) {
        }
        return itemView;
    }
}

I don't think this is a right answer: 1st textviews are not attached to the inflated layout. 2nd more content and explanation will be very appreciate. Thanks. — Blo, Apr 18 '14 at 11:29

score 1 · Accepted Answer · edited May 23 '17 at 12:24

Explanation
The issue occurs because you set the inflated layout only once, when your condition of convertView is null. However, after the first time itemView ins't null. Your items are not updated except the first one. Also, you need to return all the views (itemView) for the list items.

Solution
In your adapter, the getView method needs to inflate a layout (once) and returns this layout:

// inflate the layout to the row (items)
View itemView = convertView;
if(null == itemView) {
    itemView = getLayoutInflater().inflate(R.layout.item_crossroad, parent, false);
}
// Then return the inflated view and all its stuff
return itemView;

Also, you need to attach all the item's views to the layout as follows:

 // itemView.findViewById(...) to retrieve the id which is on the inflated layout
 TextView name = (TextView) itemView.findViewById(R.id.item_crossroadName);

Finally, to improve the performance of your Adapter, you should use a ViewHolder. This will avoid you to call ever and ever all the findViewById methods. Here's a great tutorial and a simple example. Then, your method might be:

 // Init your variables
 public ViewHolder {
     TextView name, redLight, greenLight
 }

 @Override
 public View getView(int position, View convertView, ViewGroup parent) {
      View itemView = convertView;
      // create the ViewHolder variable
      ViewHolderItem viewHolder;

      if(itemView == null) {
           itemView = getLayoutInflater().inflate(R.layout.item_crossroad, parent, false);

           // create a new ViewHolder
           viewHolder = new ViewHolderItem();

           // get your views with the ViewHolder and attached to the inflated layout
           viewHolder.name = (TextView) itemView.findViewById(R.id.item_crossroadName);
           viewHolder.redLight = (TextView) itemView.findViewById(R.id.item_redLightDuration);
           viewHolder.greenLight = (TextView) itemView.findViewById(R.id.item_greenLightDuration);

           // store the ViewHolder
           itemView.setTag(viewHolder);
     }else{
         // this is not the first, reuse the stored ViewHolder
         viewHolder = (ViewHolderItem) convertView.getTag();
     }

     // do your stuff...
     Crossroad currentCrossroad = crossroads.get(position);
     try {
         viewHolder.name.setText(currentCrossroad.getName());
         viewHolder.redLight.setText("" + currentCrossroad.getRedLight());
         viewHolder.greenLight.setText("" + currentCrossroad.getGreenLight());
     } catch (Exception e) { }

     // return the layout inflated
     return itemView;
}

score 0 · Answer 3 · answered Apr 18 '14 at 11:37

change the getview method to

public View getView(int position, View convertView, ViewGroup parent) {
View itemView = convertView;
if(null == itemView) {
    itemView = getLayoutInflater().inflate(R.layout.item_crossroad, parent, false);
}
    Crossroad currentCrossroad = crossroads.get(position);

    TextView name = (TextView) itemView.findViewById(R.id.item_crossroadName);
    TextView redLight = (TextView) itemView.findViewById(R.id.item_redLightDuration);
    TextView greenLight = (TextView) itemView.findViewById(R.id.item_greenLightDuration);

    name.setText(currentCrossroad.getName());
    redLight.setText("" + currentCrossroad.getRedLight());
    greenLight.setText("" + currentCrossroad.getGreenLight());

    return itemView;

}

comment me about result

Android ListView adapter loads wrong

3 Answers3