1

I've created a ListView with a custom adapter which extends the SimpleAdapter. The fifth item is pink. When I swipe up and down, other items gets random the same background-color as the fifth one.
It seems very strange to me, why is that?

Screenshots:
Screenshot 1: the app directly after start
Screenshot 2: the App after I swiped up and down a few times

Files:
MainActivity.java:

package com.example.test;

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

import android.app.ListActivity;
import android.os.Bundle;
import android.widget.ListAdapter;

public class MainActivity extends ListActivity {
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        List<Car> cars = getData();
        ListAdapter adapter = new CarListAdapter(this, cars, android.R.layout.simple_list_item_2, new String[] {
                Car.KEY_MODEL, Car.KEY_MAKE }, new int[] { android.R.id.text1, android.R.id.text2 });
        this.setListAdapter(adapter);
    }

    private List<Car> getData() {
        List<Car> cars = new ArrayList<Car>();
        for(int i = 0; i < 15; i++) {
            cars.add(new Car("Car "+i, i+""));
        }
        return cars;
    }
}



CarListAdapter.java:

package com.example.test;

import java.util.List;
import java.util.Map;

import android.content.Context;
import android.view.View;
import android.view.ViewGroup;
import android.widget.SimpleAdapter;

public class CarListAdapter extends SimpleAdapter {
    private List<Car> cars;
    private int[] colors = new int[] { 0x30ff2020, 0x30ff2020, 0x30ff2020 };

    @SuppressWarnings("unchecked")
    public CarListAdapter(Context context,
            List<? extends Map<String, String>> cars, int resource,
            String[] from, int[] to) {
        super(context, cars, resource, from, to);
        this.cars = (List<Car>) cars;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View view = super.getView(position, convertView, parent);

        if (position == 5) {
            int colorPos = position % colors.length;
            view.setBackgroundColor(colors[colorPos]);
        }

        return view;
    }

}



Car.java:

package com.example.test;

import java.util.HashMap;

public class Car extends HashMap<String, String> {
    private static final long serialVersionUID = 12872473L;
    public String make;
    public String model;

    public static String KEY_MAKE = "make";
    public static String KEY_MODEL = "model";

    public Car(String make, String model) {
        this.make = make;
        this.model = model;
    }

    @Override
    public String get(Object k) {
        String key = (String) k;
        if (KEY_MAKE.equals(key))
            return make;
        else if (KEY_MODEL.equals(key))
            return model;
        return null;
    }
}



activity_main.xml:

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

    <ListView android:id="@android:id/list" android:layout_width="fill_parent"
        android:layout_height="fill_parent" android:visibility="visible"/>

</LinearLayout>

2 Answers2

0

It because you wrote the adapter not correctly and basically creating some bugs with the help of the view recycling mechanism on which you can read more about here. Remove this line:

 View view = super.getView(position, convertView, parent);

And write this instead:

View row;
LayoutInflater inflator = (LayoutInflater)getSystemService(Context.LAYOUT_INFLATER_SERVICE);
row = inflator.inflate(R.layout.new_row, null);
if (position == 5) {
        int colorPos = position % colors.length;
        row .setBackgroundColor(colors[colorPos]);
}

return row ;

UPDATE:

When you use the getView method, you need to create a layout for your row in the layouts folder and use as the layout for every row. during the run of getView(a method that runs for each and every row that is created in your ListView) your can make the desired changed for specific rows based on their position in the list or other data attached to a list item at specific location you do this (preferably by creating a ViewHolder that will describe the visual aspect of your row from the stand point of Views) like this:

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    View row;
    final ViewHolder holder = new ViewHolder();
    LayoutInflater inflator = (LayoutInflater)getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    row = inflator.inflate(R.layout.new_row, null);
    holder.tvId = (TextView) row.findViewById(R.id.text_id);
    holder.tvId.setText(position);
    ...

    if (position == 5) {
        int colorPos = position % colors.length;
        row .setBackgroundColor(colors[colorPos]);
    }
    return row ;
}

static class ViewHolder
{
  TextView tvId;
  TextView tvTitle;
}

UPDATE2: I guess as were said in the comments this would be a better option:

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    final ViewHolder holder;
    if (convertView == null) {
        holder = new ViewHolder();
        LayoutInflater inflator = (LayoutInflater)getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        convertView = inflator.inflate(R.layout.list, null);
        holder.tvId = (TextView) convertView.findViewById(R.id.text_id);
        ...
        if (position == 5) {
           int colorPos = position % colors.length;
           convertView.setBackgroundColor(colors[colorPos]);
        }
        convertView.setTag(holder);
    }
    else {
        holder = (ViewHolder) convertView.getTag();
    }

    holder.tvId.setText(position);
    ...

    return convertView;
}

static class ViewHolder
{
  TextView tvId;
  TextView tvTitle;
}

But I found this problematic and not always working properly.

Community
  • 1
  • 1
Emil Adz
  • 40,709
  • 36
  • 140
  • 187
  • Error: _The method getSystemService(String) is undefined for the type CarListAdapter_ at LayoutInflater inflator = (LayoutInflater) getSystemService(Context.LAYOUT_INFLATER_SERVICE); –  Jun 08 '13 at 15:48
  • getSystemService is a method of an object that has context, like an activity. so you need to pass an activity to the adapter, or to use the context object that you are passing. like context.getSystemService – Emil Adz Jun 08 '13 at 15:51
  • Yeah works, but now the text is invisible/not here: [Screenshot](http://www.m-i-u.de/images-i66577b2hbea.png) –  Jun 08 '13 at 15:59
  • 1
    By doing this you lose all the benefits of view recycling. **You should only inflate a new row if `convertView` is null**. The trick is to set all properties that could have changed. For example, you need to set the **default** background color if `position != 5` – nicopico Jun 08 '13 at 16:02
  • @nicopico, you are right, but I found from my experience that using corvertView makes a lot of the same bugs as describes in the question here, maybe you are right about the " The trick is to set all properties that could have changed" but doing that I couldn't eliminate the bugs. and if the numbers of list elements is not that big, it doesn't have any difference. – Emil Adz Jun 08 '13 at 16:12
  • @nicopico I was about to answer with the `else`. Can you move it to an answer? – MaciejGórski Jun 08 '13 at 16:12
  • Holder pattern makes no sense if you don't reuse views. – MaciejGórski Jun 08 '13 at 16:15
  • @MaciejGórski, see updated number 2, you think this is the right way? – Emil Adz Jun 08 '13 at 16:29
  • 1
    @EmilAdz No. Your answer doesn't bring anything but problems we can see in Auroratic's comments. This code is for when `BaseAdapter` is used, but in this case we see `SimpleAdapter` extended, which already does fine job of reusing views and setting all the values on subviews. The correct answer is what nicopico said: handling the case where `position != 5` with `else` clause. – MaciejGórski Jun 08 '13 at 16:36
  • @MaciejGórski seems to work fine now, thanks! .. I'll update my question –  Jun 08 '13 at 16:46
0

in the getView, you didn't handle the other cases (when it's not equals to 5) :

  if (position == 5) {
            int colorPos = position % colors.length;
            view.setBackgroundColor(colors[colorPos]);
        }
  else view.setBackgroundColor(defaultColor);

the reason is that it uses old views to be re-used .

also, you should use android:cacheColorHint="#00000000" . here's an article about it: http://www.curious-creature.org/2008/12/22/why-is-my-list-black-an-android-optimization/

android developer
  • 114,585
  • 152
  • 739
  • 1,270
  • yes, see the comments at the other [answer](http://stackoverflow.com/questions/17001061/listview-item-get-background-color-of-another-at-swipe/17004415?noredirect=1#comment24565540_17001157) –  Jun 08 '13 at 22:27