2
\$\begingroup\$

I need to get values from a Map by Adapter position i get from a RecyclerView.

As you can see each time i click on an Album Art i create a new array of album objects.

final Album[] albums = new Album[albumMap.size()];
      for (Map.Entry<Integer,Album> e : albumMap.entrySet()){
      albums[i++] = e.getValue();
 }

Then i get the album like this: String selectedAlbum = albums[position].getAlbum();

But what if someone has like 10000+ albums on his device and each time an album is clicked a new array of album objects is created and i iterate through it just to get the album name.

Would this have an impact on performance if there are alot of albums present?

TL;DR Is this code bad?

Full code

@Override
    public void onClickAlbum(int position, Map<Integer,Album> albumMap) {
        if (getActivity() != null) {
            int i = 0;
            final Album[] albums = new Album[albumMap.size()];
            for (Map.Entry<Integer,Album> e : albumMap.entrySet()){
                albums[i++] = e.getValue();
            }
            String selectedAlbum = albums[position].getAlbum();
            Main.getInstance().setSongsFilteredBy(SongsLibrary.getInstance().getSongsByAlbum(selectedAlbum));
            Intent intent = new Intent(getActivity(), ListSongsActivity.class);
            intent.putExtra("title", selectedAlbum);
            startActivity(intent);
            Toast.makeText(getActivity(), "test: " + selectedAlbum, Toast.LENGTH_SHORT).show();
        }
}
\$\endgroup\$
  • \$\begingroup\$ Of course that will give you a performance hit, but maybe less than you expect. Check how hash maps affect performance. BTW 10000 is a mickey mouse number of records. \$\endgroup\$ – πάντα ῥεῖ Apr 4 at 10:14
2
\$\begingroup\$

Technical improvements

  • use albumMap.values() to get a Collection<Album> directly instantiated as list, which supports positional access get(index). So no need for array and filling loop. See Listing 1.
  • use some of Android's ListView interfaces which usually provide a convenient method on their underlying data model like getItemAtPosition(position). See Listing 2.

Listing 1: Q: How to convert map to list

List<Album> albums = new ArrayList<Album>(albumMap.values());

Listing 2: Vogella's Tutorial on Android's ListView

ListView albumsView = (ListView) findViewById(R.id.listview);

// fill albums data to model (adapter implementation)
albumsView.setAdapter(adapter)

albumsView.setOnItemClickListener(new OnItemClickListener() {

  @Override
  public void onItemClick(AdapterView<?> parent, final View view, int position, long id) {
    final Album album = (Album) parent.getItemAtPosition(position);
    // your implementation
  }
}

Design improvements

  • album.getAlbum() gets the title of an album, so rename property|getter to title|getTitle()
  • keep the event-handler short and extract all action/intend creation into separate method

References

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I am not familiar with Android so I don't know how you get albumMap into the click event. Can you get selected Album (album object) into the event?

If not..

I assume albumMap is a map between position to Album.

If so I suggest the name postion2album. if not please update your post and clarify what this map is.

So all you need to do is albumMap[position].getAlbum()

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.