4

I have a ListView in a Fragment and I want to update the data in the ListView when I return from another Activity. I have overwritten the onResume() method in the Fragment, modify the data in the Adapter and call notifyDataSetChanged() on the Adpater but somehow the ListView is not being updated. I suspect that there is something wrong with my Adapter, but I just can't seem to find the error.

Here's code of my Adpater:

class ManualExceptionsListAdapter extends BaseAdapter {

    private LayoutInflater mInflater;
    private TextView mManualExceptions;
    SwitchCompat mSwitch;
    TextView name;
    final Context context = getActivity();
    final SharedPreferences mSharedPreferences = PreferenceManager.getDefaultSharedPreferences(context);
    int a;
    int ifUse = 0;

    ManualExceptionsListAdapter(LayoutInflater inflater) {
        mInflater = inflater;
    }

    @Override
    public int getCount() {
        return (mPermanentManualException.size()+mContactsExceptionNumber.size());
    }

    @Override
    public Object getItem(int i) {
        return null;
    }

    @Override
    public long getItemId(int i) {
        return 0;
    }

    @Override
    public int getItemViewType(int position) {
        if (position < (mContactsExceptionNumber.size())) {
            a = 0;
            if(position == (mContactsExceptionNumber.size()-1)){
                ifUse = 1;
            }
            return a;
        } else {
            a = 1;
            return a;
        }
    }

    @Override
    public int getViewTypeCount() {
        return 2;
    }

    @Override
    public void notifyDataSetChanged() {
        super.notifyDataSetChanged();
    }

    @Override
    public View getView(int i, View view, ViewGroup viewGroup) {
        final int pos;
        if(mContactsExceptionNumber.size()>0) {
            pos = i - (mContactsExceptionNumber.size());
        }else{
            pos = 0;
        }
        int pos2 = 0;
        int type = getItemViewType(i);
        if(ifUse == 1){
            if(mContactsExceptionNumber.size()>0) {
                pos2 = i - (mContactsExceptionNumber.size());
                Exceptions.index = pos2;
            }
        }
        View v = view;
        if (view == null) {
            switch (type) {
                case 0:
                    v = mInflater.inflate(R.layout.contacts_exception_row, null);
                    name = (TextView) v.findViewById(R.id.contact_name);
                    name.setText(mContactsExceptionNames.get(i));
                    break;
                case 1:
                    v = mInflater.inflate(R.layout.manual_exception_row, null);
                    mManualExceptions = (TextView) v.findViewById(R.id.manual_exception_number);
                    mSwitch = (SwitchCompat) v.findViewById(R.id.manual_exception_switch);
                    mManualExceptions.setText(mPermanentManualException.get(pos2));
                    mSwitch.setTag(i);
                    try {
                        if (mManualExceptionList.contains(mPermanentManualException.get(pos2))) {
                            mSwitch.setChecked(true);
                        }
                    } catch (Exception e) {
                        e.printStackTrace();
                    }
                    break;
            }
        }else{
            switch (type) {
                case 0:
                    v = mInflater.inflate(R.layout.contacts_exception_row, null);
                    name = (TextView) v.findViewById(R.id.contact_name);
                    name.setText(mContactsExceptionNames.get(i));
                    break;
                case 1:
                    v = mInflater.inflate(R.layout.manual_exception_row, null);
                    mManualExceptions = (TextView) v.findViewById(R.id.manual_exception_number);
                    mSwitch = (SwitchCompat) v.findViewById(R.id.manual_exception_switch);
                    mManualExceptions.setText(mPermanentManualException.get(pos2));
                    mSwitch.setTag(i);
                    try {
                        if (mManualExceptionList.contains(mPermanentManualException.get(pos2))) {
                            mSwitch.setChecked(true);
                        }
                    } catch (Exception e) {
                        e.printStackTrace();
                    }
                    break;
            }
        }

        try {
            mSwitch.setOnTouchListener(new View.OnTouchListener() {
                @Override
                public boolean onTouch(View view, MotionEvent motionEvent) {
                    isTouched = true;
                    return false;
                }
            });
        } catch (NullPointerException e) {
            e.printStackTrace();
        }
        try {
            mSwitch.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
                @Override
                public void onCheckedChanged(CompoundButton compoundButton, boolean b) {
                    if (isTouched) {
                        if (b) {
                            if (!mManualExceptionList.contains((mPermanentManualException.get(pos)))) {
                                mManualExceptionList.add((mPermanentManualException.get(pos)));
                            }
                            mSharedPreferences.edit().putString("ManualExceptions", TextUtils.
                                    join(",", mManualExceptionList)).apply();

                        } else {
                            try {
                                mManualExceptionList.remove((mPermanentManualException.get(pos)));
                                mSharedPreferences.edit().putString("ManualExceptions", TextUtils.
                                        join(",", mManualExceptionList)).apply();
                            } catch (Exception e) {
                                e.printStackTrace();
                            }
                        }
                        Log.d("RejectCall", "Permanent " + TextUtils.join(",", mPermanentManualException));
                        Log.d("RejectCall", TextUtils.join(",", mManualExceptionList));
                    }
                }
            });
        } catch (NullPointerException e) {
            e.printStackTrace();
        }

        return v;
    }

}
Mahendran
  • 2,719
  • 5
  • 28
  • 50
Slay
  • 730
  • 1
  • 7
  • 18
  • `onResume()` is called upon returning to the previous `Activity`/`Fragment`. – Xaver Kapeller Nov 30 '14 at 22:45
  • @XaverKapeller That's great. Thanks. Anyways, I called notifydatasetchanged withing onResume, but the listview won't update. – Slay Nov 30 '14 at 22:49
  • Are you overriding `onResume()` of one of the `Fragments` inside the `ViewPager`? Because that won't work. Override `onResume()` in the `Fragment` or `Activity` which contains the `ViewPager`. – Xaver Kapeller Nov 30 '14 at 22:52
  • I am overriding it in one of the fragments of viewpager, and it is working. By working I mean it is being called. Should I be doing it the way you just suggested? – Slay Nov 30 '14 at 22:54
  • Well if `onResume()` is being called then there seems to be another unrelated issue with the `ListView` that is keeping it from being updated. Are you sure you change the data inside the `Adapter`? – Xaver Kapeller Nov 30 '14 at 22:56
  • @XaverKapeller Yes, I do. Because when I close the app and open it up again, the list is updated. – Slay Nov 30 '14 at 22:57
  • That doesn't mean anything, are you really sure you change the data in the `Adapter`? You can also try to create a new `Adapter` with the new data instead of updating the data in the old one. – Xaver Kapeller Nov 30 '14 at 23:01
  • Are you using a custom `Adapter` or something like an `ArrayAdapter`? – Xaver Kapeller Nov 30 '14 at 23:02
  • I'm using a BaseAdapter. Should I just recreate it withing onResume? Would that affect the performance significatnly? – Slay Nov 30 '14 at 23:03
  • If it works when you recreate the `Adapter` then you know that there is an issue in the `Adapter` itself which causes `notifyDatasetChanged()` to not work. And it doesn't impact the performance in any way. – Xaver Kapeller Nov 30 '14 at 23:05
  • @XaverKapeller recreating the adapter doesn't work either. Would you mind taking a look at the adapter class? I've updated the description of the post. – Slay Nov 30 '14 at 23:12
  • I am going to look through the code now, give me a few minutes. On first glance there are a few issues with the way you implement this `Adapter`. – Xaver Kapeller Nov 30 '14 at 23:23
  • @XaverKapeller Yup, it's not the most elegant code you'll read. Sorry about that. Could you please point out where I can make it better. – Slay Nov 30 '14 at 23:25
  • Your code is kind of complex and there are a lot of out of scope variables you use so I cannot fix your code, but I will write up an answer explaining how to implement `Adapters` like this and you can then apply that to your `Adapter`. To summarise: You are not using the view holder pattern, you keep a global reference to a `View` which can seriously mess up the view recycling and there appear to be virtually no separation between view model and view holder. I'm going to need some time to write up an answer but I'm gonna start right now. – Xaver Kapeller Nov 30 '14 at 23:30
  • Needless to say that you should use this opportunity and start using the `RecyclerView` instead of the old `ListView`. – Xaver Kapeller Nov 30 '14 at 23:31
  • @XaverKapeller Yup, I've been meaning to change it to RecyclerView. I will. Also, thank you for helping out. I'm curious to look at better way of implementing this adapter. – Slay Nov 30 '14 at 23:34
  • Are you at all familiar with the process of view recycling which is used by the `ListView`? – Xaver Kapeller Nov 30 '14 at 23:48
  • @XaverKapeller I wasn't very clear until a few minutes back, but after you told me I was doing a lot of things in an inefficient way, I read up on it. I now understand why viewholder is important. But I should mention that I'm using two listviews with the same adapter. That is, I've merged two listviews within one adapter. So I'm not really sure how I'd implement a viewholder in this case. Should I be creating two viewholder classes? – Slay Nov 30 '14 at 23:57
  • Two `ListViews` with the same `Adapter`? That is normally a recipe for disaster. Each `ListView` should have its own `Adapter`. Also see my answer for an explanation on how to efficiently implement `Adapters`. – Xaver Kapeller Dec 01 '14 at 00:31

1 Answers1

9

There are multiple issues with you Adapter implementation. Too many for me to give you advice on how to fix it. I am just going to explain how you can efficiently implement an Adapter and you can then apply this to your Adapter.

Suffice it to say that you should optimally switch to using the new RecyclerView which has many major improvements over the old ListView. You can find the RecyclerView documentation here and Googles guide on how to use it here.


If you want to display data in a ListView you should first create the layout for each of the items in the ListView. For this example I will use this layout:

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:padding="12dp">

    <CheckBox
        android:id="@+id/checkbox"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"/>

</RelativeLayout>

To bundle the data we want to display in each item of the ListView we write a new class which just contains the data in private fields and getters and setters to get and set that data. Such classes are usually called view models. A view model for a layout like above might look something like this:

public class ExampleViewModel {

    // This is the text which will be set to the CheckBox
    private String text;

    // This boolean will be used to save the checked state of the CheckBox
    private boolean checked;

    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    public boolean isChecked() {
        return checked;
    }

    public void setChecked(boolean checked) {
        this.checked = checked;
    }
}

Each instance of such a view model would represent one item in the ListView. When one of the items enters the visible area of the ListView it has to be bound to a View in the ListView (That is something we have to implement in getView() of the Adapter). As long as the item is visible the model will stay bound to this one View, but as soon as the View has exited the visible area of the ListView it will be recycled and bound to a different view model which is just entering the visible area. This is called view recycling and it is done to minimise the memory footprint of the ListView and to increase overall scroll performance and fluidity. Views are very expensive objects, especially inflating Views and findViewById() cost a lot of performance and the main point of view recycling is that you have to inflate only a small number of Views once which can then be reused and therefore you avoid the expensive inflating and findViewById() later on.

Most of what I explained above happens automatically. What you as a developer have to do is inflate the correct Views in getView() or reuse them if there already is one available and then bind the correct view model to the View. I know that most of this seems rather complicated and confusing if you first hear about it, but it gets much simpler and more obvious once we start to look at the code.

So now we have the layout of the view items in the ListView and the view model to go along with it. What we need to do now is write another class which are usually called view holders. These view holders are essentially container classes around the views in the ListView. Each view holder contains one View associated with an item in the ListView and they also take care of binding the data of the view model to the View. Without further ado here is a view holder to go along with the view model from above:

public class ExampleViewHolder {

    // The reference to the CheckBox is saved so we only have to perform the findViewById() once.
    private final CheckBox checkBox;

    // A reference to the view model which is currently bound to this view holder
    private ExampleViewModel currentModel;

    // The View associated with this view holder is passed into the constructor from the Adapter.
    public ExampleViewHolder(View view) {

        // And here we look for all relevant views
        // In our case we just need the CheckBox
        this.checkBox = (CheckBox) view.findViewById(R.id.checkbox);
    }

    public void bind(ExampleViewModel viewModel) {

        // Unset the listener in case there was one from a previous view model
        this.checkBox.setOnCheckedChangeListener(null);

        // Save a reference to the view model which is currently bound to this view holder
        this.currentModel = viewModel;

        // Bind the data to the CheckBox
        this.checkBox.setText(viewModel.getText());
        this.checkBox.setChecked(viewModel.isChecked());

        // Reset the listener
        this.checkBox.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
            @Override
            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
                currentModel.setChecked(isChecked);
            }
        });
    }
}

Now we are almost finished. The only thing that is missing now is to plug all this together in the Adapter:

public class ExampleAdapter extends BaseAdapter {

    // Each type of view in the `ListView` gets its own id
    // In this example we only have one type of View so we only need one id
    private static final int EXAMPLE_VIEW_ID = 0;

    // The default view id is just a fallback
    private static final int DEFAULT_VIEW_ID = EXAMPLE_VIEW_ID;

    private final LayoutInflater inflater;
    private List<ExampleViewModel> viewModels;

    public ExampleAdapter(Context context, List<ExampleViewModel> viewModels) {

        // The view models are initially passed in through the constructor.
        // You can pass an empty list into the Adapter if there is not data initially.
        this.viewModels = viewModels;
        this.inflater = LayoutInflater.from(context);
    }

    @Override
    public int getCount() {
        if(viewModels == null) {
            return 0;
        }

        return viewModels.size();
    }

    @Override
    public Object getItem(int position) {
        return viewModels.get(position);
    }

    @Override
    public long getItemId(int position) {

        final Object model = getItem(position);

        // Here we check if the model is an instance of ExampleViewModel and if yes we return its id
        if(model instanceof ExampleViewModel) {
            return EXAMPLE_VIEW_ID;
        }

        return DEFAULT_VIEW_ID;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {

        if(getItemId(position) == EXAMPLE_VIEW_ID) {
            final ExampleViewModel model = (ExampleViewModel) getItem(position);

            final ExampleViewHolder viewHolder;

            // If the convertView is null we need to inflate a new view
            if(convertView == null) {
                final View view = this.inflater.inflate(ExampleViewHolder.LAYOUT, parent, false);   
                viewHolder = new ExampleViewHolder(view);

                // Here we set the viewHolder as tag to the View
                // This is done so we can reuse the same view holder later on
                // Essentially this is the integral part of the whole view recycling process
                view.setTag(viewHolder);
            } else {
                // If the convertView is not null we can just get the view holder with getTag() from the View
                viewHolder = (ExampleViewHolder) convertView.getTag();
            }

            // And we just need to bind the model to the view holder
            viewHolder.bind(model);
        }

        return convertView;
    }
}

And that is all you need. This is pretty much a best practice implementation of an Adapter. If you are dealing with two or more different types of views you need to write a view model and view holder class for each type. You can write an interface called ViewModel which would look something like this:

public interface ViewModel {

}

Every view model of yours should implement this interface. You can then use List<ViewModel> in the Adapter which can contain all different kinds of view models.

public class TypeOneViewModel implements ViewModel {

}

public class TypeTwoViewModel implements ViewModel {

}

As soon as all your view models implement this interface you can do this:

final List<ViewModel> models = new ArrayList<ViewModel>();
models.add(new TypeOneViewModel());
models.add(new TypeTwoViewModel());
...

And this List which now contains multiple different types of view models can then be passed to the Adapter. The Adapter would then look something like this:

public class ExampleAdapter extends BaseAdapter {

    private static final int TYPE_ONE_VIEW_ID = 0;
    private static final int TYPE_TWO_VIEW_ID = 1;
    private static final int DEFAULT_VIEW_ID = TYPE_ONE_VIEW_ID;

    private final LayoutInflater inflater;
    private List<ViewModel> viewModels;

    public ExampleAdapter(Context context, List<ViewModel> viewModels) {
        this.viewModels = viewModels;
        this.inflater = LayoutInflater.from(context);
    }

    @Override
    public int getCount() {
        if(viewModels == null) {
            return 0;
        }

        return viewModels.size();
    }

    @Override
    public ViewModel getItem(int position) {
        return viewModels.get(position);
    }

    @Override
    public long getItemId(int position) {

        final ViewModel model = getItem(position);

        if(model instanceof TypeOneViewModel) {
            return TYPE_ONE_VIEW_ID;
        }

        if(model instanceof TypeTwoViewModel) {
            return TYPE_TWO_VIEW_ID;
        }

        return DEFAULT_VIEW_ID;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {

        if(getItemId(position) == TYPE_ONE_VIEW_ID) {
            final TypeOneViewModel model = (TypeOneViewModel) getItem(position);

            final TypeOneViewHolder viewHolder;
            if(convertView == null) {
                final View view = this.inflater.inflate(TypeOneViewHolder.LAYOUT, parent, false);
                viewHolder = new TypeOneViewHolder(view);
                view.setTag(viewHolder);
            } else {
                viewHolder = (TypeOneViewHolder) convertView.getTag();
            }

            viewHolder.bind(model);
        }

        if(getItemId(position) == TYPE_TWO_VIEW_ID) {
            final TypeTwoViewModel model = (TypeTwoViewModel) getItem(position);

            final TypeTwoViewHolder viewHolder;
            if(convertView == null) {
                final View view = this.inflater.inflate(TypeTwoViewHolder.LAYOUT, parent, false);
                viewHolder = new TypeTwoViewHolder(view);
                view.setTag(viewHolder);
            } else {
                viewHolder = (TypeTwoViewHolder) convertView.getTag();
            }

            viewHolder.bind(model);
        }

        return convertView;
    }
}

You can also unify your view holders by writing an abstract class. Such an abstract class would look something like this:

public abstract class ViewHolder<T extends ViewModel> {
    protected final View itemView;

    public ViewHolder(View view) {
        this.itemView = view;
    }

    public abstract void bind(T model);
}

If you use this abstract class as base class for your view holders you would write them like this:

public class TypeOneViewHolder extends ViewHolder<TypeOneViewModel> {
    public TypeOneViewHolder(View view) {
        super(view);

        ...
    }

    public void bind(TypeOneViewModel model) {
        ...
    }
}

Although this part isn't really required. The most important part when dealing with multiple different types of items in the ListView is that all the models implement a common interface so you can safely put them in the same List.

Anyway, this whole thing looks a lot simpler and cleaner than your Adapter, doesn't it? This way you have perfect separation between the data in the ListView and the Views which display the data and it's a lot more maintainable. You can easily implement animations in the view holder without having to concern yourself with the view recycling and a lot of requirements become a lot simpler to implement. The RecyclerView takes all this to the next level of course. It works in much the same way, but has several major improvements over the ListView, I really suggest you take a look at it.


One thing I completely forgot: You can expose the internal List of view models with a getter so you can modify the view models from the outside. Add methods like this to the Adapter:

public List<ExampleViewModel> viewmodels() {
    return viewModels;
}

public void setViewModels(List<ExampleViewModel> models) {
    viewModels = models;
}

Then you can modify the view models in the Adapter like this:

adapter.setViewModels(newData);
...
adapter.viewmodels().add(viewModel);

And when you are finished modifying the data you can update the ListView by calling notifyDataSetChanged() on the Adapter.

Xaver Kapeller
  • 49,491
  • 11
  • 98
  • 86
  • This is so simplified. You've gone to the nuts and bolts and made it clear. Thank you! Also, if I want a single listview showing two different types of rows, should I be creating two different adapters? Because I've done it using just one adapter. – Slay Dec 01 '14 at 00:56
  • Towards the bottom of my answer I show how to implement multiple types of items in the same `Adapter` if that's what you mean. – Xaver Kapeller Dec 01 '14 at 00:59
  • Didn't quite understand that part. What exactly is a model in this context? – Slay Dec 01 '14 at 01:02
  • What are you referring to? – Xaver Kapeller Dec 01 '14 at 01:08
  • I was referring to `List models`. I'll read up on it. Thank you for helping out! – Slay Dec 01 '14 at 08:31
  • Would it be ok for you if I edit your question to bring it more in line with the answer? This would create a question/answer pair which would be more suited for this site. – Xaver Kapeller Dec 01 '14 at 11:15
  • I was actually just thinking about it, since this post has deviated from the original question, and now has an in-depth explanation to how adapters work. This post of yours could really help a lot of people. Please do it to make it more relevant to your post. – Slay Dec 01 '14 at 11:25
  • Thanks @XaverKapeller !! This was an awesome start for a person like starting with Android... And 2 thumbs up for renaming the question to bring it in context. – Ramanpreet Singh Jun 05 '15 at 17:34
  • Thanks @XaverKapeller this QA deserve to be popular. Especially ViewHolder has no access to data model and you bound it. Which is the common case when you implement list-detail pattern. – Mahendran Aug 25 '15 at 06:28
  • In your basic getView implementation, you return convertView even if it's null, and no the view that you created. Is that the correct implementation? And if so how does your newly created view get displayed/what is the meaning of the return value? – Dean Gurvitz Dec 30 '18 at 08:21