Create albums out of songs

Question

I have made a method which returns a list of all songs on my device. Now I want to get a list of albums out of these songs.

What I basically do is go through every song I have in my list, check if its album is already in the list and if not I create a new album.

This method will take some time since there are 1400 songs on my device so I wondered how I can make this method any faster? Also, any other improvements?

public static ArrayList<Album> loadAlbums( Context c ){
    ArrayList<Song> tmpList = getAllSongs( c);
    ArrayList<Album> albumList = new ArrayList<Album>();

    for( Song s : tmpList ){

        boolean added = false;

        for( Album a : albumList ){

            if( a.getAlbumName().contentEquals( s.getAlbum() ) && a.getArtistName().contentEquals( s.getArtist() ) ){
                a.addSong( s.getPath() );
                added = true;
            }
        }


        if( !added ){
            Album a = new Album( s.getAlbum() );
            a.setAlbum_id( s.getAlbumId() );
            a.setArtistName( s.getArtist() );
            a.addSong( s.getPath() );

            albumList.add(a);
        }

    }

    tmpList = null;
    return albumList;
}

Answer 1

What you need is a Map. A map is a collection of key --> value combinations. There can not be two equal keys pointing to different values.

There are a few ways to organize this map. The simplest is to make Map<String, Album> albumNames;, but that would make it possible for collisions if two albums had the same album name.

Another alternative is Map<String, List<Album>> for fixing the problem with albums having the same name.

There are more alternatives but they can get quite complex so I think this is enough for now.

Then when you loop through your songs, for each song:

• List<Album> possibleAlbums = albumNames.get(song.getAlbum()).
  • If that returns null, then add a new list and add an album to it.
  • otherwise loop through the list to find the album you are looking for (that matches the artist). Again, if none is found create a new one.
• Once you have found or created a matching album, you can add the song to it.

Other comments:

• Declare by interface and not implementation. Make the method return List<Album>, also use for example List<Song> tmpList = getAllSongs(c);

• Use longer and better names. s --> song, a --> album, c --> context, tmpList --> songs.

• tmpList = null; in the bottom of the method will have no effect whatsoever. You can safely remove that line.

Answer 2

• The loadAlbums method should probably take a list of songs as its parameter instead of the less meaningful Context object.

• The name getAlbum makes me thing that the method is going to return an object of type Album, so that should be renamed to getAlbumName().

• You don't want to be constructing an album object by setting each of the object's properties line-by-line. Instead, add a static method on the album class for constructing an album object from a song:

  Album a = Album.GetAlbumFromSong(s)
  

• Use a Map object to map between Albums and an ArrayList of songs on that album: HashMap<Album, ArrayList<Song>> albums. Then you can override equals and hashCode for type Album to test album equality using artist name and album name, then you can write:

  for(Song song: tmpList){
      Album inspectionAlbum = Album.GetAlbumFromSong(song);
      if(albums.containsKey(inspection)){
          albums.get(inspectionAlbum).add(song);
      }
  }
  

  The amount of time it takes to look up keys in your hashmap won't depend on size which should make this method a lot more performant for large input sets.

• tmpList should be renamed to songs.