SearchActivity to search through an ArrayList<Obj>

Question

I've coded this class that lets me search through an ArrayList displayed into a RecyclerView. This is the class:

public class SearchActivity extends AppCompatActivity {
 ArrayList<Accordo> chords;
 RecyclerView rv;
 SearchView sv;
 ArrayList<Accordo> filteredList;
 @Override
 protected void onCreate(@Nullable Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.search_layout);
 /** gestisce la pubblicita */
 MobileAds.initialize(getApplicationContext(), "ca-app-pub-3940256099942544/6300978111");
 AdView searchBanner = (AdView) findViewById(R.id.search_ad);
 AdRequest adRequest = new AdRequest.Builder().build();
 searchBanner.loadAd(adRequest);
 /**-------------------------------*/
 Intent intent = this.getIntent();
 Bundle bundle = intent.getExtras();
 chords = bundle.getParcelableArrayList("chords");
 filteredList = bundle.getParcelableArrayList("chords");
 sv = (SearchView) findViewById(R.id.testo_ricerca);
 sv.setIconifiedByDefault(false);
 rv = (RecyclerView) findViewById(R.id.lista_ricerca);
 rv.setLayoutManager(new LinearLayoutManager(SearchActivity.this, LinearLayoutManager.VERTICAL, false));
 rv.setHasFixedSize(true);
 final SearchAdapter adapter = new SearchAdapter(this, chords);
 rv.setAdapter(adapter);
 //SEARCH
 sv.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
 @Override
 public boolean onQueryTextSubmit(String query) {
 return false;
 }
 @Override
 public boolean onQueryTextChange(String newText) {
 //FILTER AS YOU TYPE
 List<Accordo> filteredModelList = filter(chords, newText);
 adapter.setFilter(filteredModelList);
 return true;
 }
 });
 /** gestisce cosa succede quando un elemento della lista viene cliccato */
 ItemClickSupport.addTo(rv).setOnItemClickListener(new ItemClickSupport.OnItemClickListener(){
 @Override
 public void onItemClicked(RecyclerView recyclerView, int position, View v) {
 Intent intent = new Intent(SearchActivity.this, ChordActivity.class);
 Bundle bundle = new Bundle();
 bundle.putParcelable("selected", filteredList.get(position));
 intent.putExtras(bundle);
 startActivity(intent);
 }
 });
 }
 private List<Accordo> filter(List<Accordo> models, String query) {
 query = query.toLowerCase();
 filteredList = new ArrayList<>();
 for (Accordo model : models) {
 final String text = model.getName().toLowerCase();
 if (text.contains(query)) {
 filteredList.add(model);
 }
 }
 return filteredList;
 }
}

As you can see from the code above, it is based on a filter that every time the user types something in the search box, creates a filteredList and puts it into the RecyclerView. Of course when the user did not type anything the filteredList should have contained the whole ArrayList chords

So I was getting an error (NPE) when I clicked on an Item while nothing had been typed into the search box, because the filteredList was null. I managed to solve it by having both:

chords = bundle.getParcelableArrayList("chords");

filteredList = bundle.getParcelableArrayList("chords");

I think this is a huge waste of memory and resources since we are talking about an ArrayList with ca 300 elements, each of which has 5 images, strings and sounds.

Is there a more efficient way to achieve the same result?

Answer 1

Once is enough of bundle.getParcelableArrayList("chords")

I'm not sure if bundle.getParcelableArrayList creates a new list every time it's called. If it doesn't, then repeated calls won't double the memory used so it won't really be a problem. But in any case, you don't need to call it twice like this:

chords = bundle.getParcelableArrayList("chords");
filteredList = bundle.getParcelableArrayList("chords");

You can make filteredList reference chords:

filteredList = chords = bundle.getParcelableArrayList("chords");

Use interfaces in declarations

These fields would be better declared as List instead of ArrayList:

ArrayList<Accordo> chords;
ArrayList<Accordo> filteredList;

Just like you used List instead of ArrayList in the filter method.

Also, probably all fields of the activity should be private.

Reduce memory churn

Every time the query text changes, you recreate a new list, and re-link the adapter to the new list. It might be more efficient to reuse the same list, by clearing and re-adding elements instead of creating a new list. However, there is just one tricky point, of the initial state when there is no filter yet. The adapter could initially be linked to chords, and then the first time a query text is entered, re-link it to filteredList, for example:

@Override
public boolean onQueryTextChange(String newText) {
 if (filteredList == null) {
 // first time used
 filteredList = new ArrayList<>();
 adapter.setFilter(filteredList);
 }
 filter(newText);
 return true;
}
private void filter(String query) {
 query = query.toLowerCase();
 filteredList.clear();
 for (Accordo model : chords) {
 final String text = model.getName().toLowerCase();
 if (text.contains(query)) {
 filteredList.add(model);
 }
 }
}

Notice some other related changes:

• I dropped the list parameter of filter: the filtering is always based on chords, which is a field, so the method has direct access to it, no need to pass as parameter
• Made filter return void, as now it modifies filteredList in-place

• Daniele C: Great answer!! Thank you a lot! Just one thing, if I change filteredList and chords from ArrayList to List I cannot use bundle.getParcelableArrayList, is there something I could use instead?