RecyclerView lags on scrolling

I'm having an issue where when the recyclerview has a big amount of items (say 2000) the scrolling is really laggy.

Here's the Fragment code:

```
package jahirfiquitiva.apps.iconshowcase.fragments;

import android.os.Bundle;
import android.support.v4.app.Fragment;
import android.support.v7.widget.GridLayoutManager;
import android.support.v7.widget.RecyclerView;
import android.view.InflateException;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;

import com.pluscubed.recyclerfastscroll.RecyclerFastScroller;

import java.util.ArrayList;
import java.util.Locale;

import jahirfiquitiva.apps.iconshowcase.R;
import jahirfiquitiva.apps.iconshowcase.adapters.IconsAdapter;
import jahirfiquitiva.apps.iconshowcase.utilities.Preferences;
import jp.wasabeef.recyclerview.adapters.AlphaInAnimationAdapter;
import jp.wasabeef.recyclerview.adapters.ScaleInAnimationAdapter;

public class IconsFragment extends Fragment {

private IconsAdapter mAdapter;
private Preferences mPrefs;
private ArrayList iconsNames, filteredIconsList;
private ArrayList iconsInts, filteredIconsInts;
private ViewGroup layout;

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {

mPrefs = new Preferences(getActivity());

if (layout != null) {
ViewGroup parent = (ViewGroup) layout.getParent();
if (parent != null) {
parent.removeView(layout);
}
}
try {
layout = (ViewGroup) inflater.inflate(R.layout.icons_grid, container, false);
} catch (InflateException e) {

}

RecyclerFastScroller fastScroller =
(RecyclerFastScroller) layout.findViewById(R.id.rvFastScroller);
fastScroller.setVisibility(View.GONE);

RecyclerView iconsGrid = (RecyclerView) layout.findViewById(R.id.iconsGrid);
iconsGrid.setHasFixedSize(true);
iconsGri

private synchronized void filter(CharSequence s, IconsAdapter adapter) {
    if (s == null || s.toString().trim().isEmpty()) {
        if (filteredIconsList != null) {
            filteredIconsList = null;
        }
        if (filteredIconsInts != null) {
            filteredIconsList = null;
        }
        ...

This section of code, you check if the filteredIconsList is not null. If so, set it to null. The only value which is going to equal to null is null, so skip the check and just set it to null.

private synchronized void filter(CharSequence s, IconsAdapter adapter) {
    if (s == null || s.toString().trim().isEmpty()) {
        filteredIconsList = null;
        if (filteredIconsInts != null) {
            filteredIconsList = null;
        }
        ...

Which makes the next if statement just redundant. filteredIconsList is already null. So remove that if statement.

adapter.notifyDataSetChanged();
    }
}

If both cases end with adapter.notifyDataSetChanged(), then you can just move the line to outside of the if statement.

In the else case of this if statement, you have this:

if (filteredIconsInts != null) {
            filteredIconsList = null;
        }
        filteredIconsList = new ArrayList();

That's a redundant set, you don't need to set to null if you're going to overwrite it with a new ArrayList just afterwards.

So remove the useless if-statement.

In the for loop, you retrieve the name with iconsNames.get(i), but if the condition in the for loop is true, you iconsNames.get(i) to filteredIconsList. Just add name to the filteredIconsList, as you've already retrieved it.

The documentation for toLowerCase says this:


public String toLowerCase()

Converts all of the characters in this String to lower case using the rules of the default locale. This is equivalent to calling toLowerCase(Locale.getDefault()).

So there's no need to include the Locale. Remove the argument.

Doing all these toLowerCase in a loop is pretty silly, though. Move it to outside of the loop, so you only lowercase once.

Results in this:

private synchronized void filter(CharSequence s, IconsAdapter adapter) {
    if (s == null || s.toString().trim().isEmpty()) {
        filteredIconsList = null;
        adapter.clearIconsList();
        adapter.setIcons(iconsNames, iconsInts);
    } else {
        if (filteredIconsList != null) {
            filteredIconsList.clear();
        }
        filteredIconsList = new ArrayList();
        filteredIconsInts = new ArrayList();
        String lowerCaseS = s.toString.toLowerCase();
        for (int i = 0; i < iconsNames.size(); i++) {
            String name = iconsNames.get(i);
            if (name.toLowerCase()
                    .startsWith(lowerCaseS)) {
                filteredIconsList.add(name);
                filteredIconsInts.add(iconsInts.get(i));
            }
        }
        adapter.clearIconsList();
        adapter.setIcons(filteredIconsList, filteredIconsInts);
    }
    adapter.notifyDataSetChanged();
}

If we're allowed to change the order of operations, I'd even move the adapter.clearIconsList() all the way to the top of the function.

StackExchange Code Review Q#114419, answer score: 2