Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

Java Java Data Structures - Retired Efficiency! Implement Chooser UI

Posted by Adiv Abramson
Adiv Abramson
6,919 Points

Why try to return a list of songs and then test if the result is null when you could use containsKey()?

It seems wasteful to try and retrieve a list of songs as a way to test for the existence of an artist as the key in a map. We can use the containsKey() method instead and if it returns true we can add to the list of songs in the value slot corresponding to that artist in the HashMap.

for (Song song : mSongs) {

String strArtist = song.getArtist();
String strTitle = song.getTitle();
List<String> artistSongs = new ArrayList<>();

If (byArtist.containsKey(strArtist) {
  //get the existing list of songs for this artist in the Map
  artistSongs = byArtist.get(strArtist);
  //add to the list of songs for this artist
  artistSongs.add(strTitle);

  //update the Map
   byArtist.set(strArtist, artistSongs);


} else {
   //create a new entry in the Map for this artist
   artistSongs.add(strTitle);
   byArtist.put(strArtist, artistSongs);
}//If (byArtist.containsKey(strArtist) 
}//for (Song song : mSongs)

Would this code work correctly in this context? Is it a better method or are there other problems I fail to recognize, assuming I got the syntax correct? LOL

Thanks very much

1 Answer

Dan Johnson
Dan Johnson
40,533 Points
Dan Johnson
Dan Johnson
40,533 Points

The containsKey method is a valid way to go about it, and more expressive too. Here's a solution close to what you were going for:

for (Song song : mSongs) {
    String artist = song.getArtist();
    List<Song> songs;

    if (byArtist.containsKey(artist)) {
        songs = byArtist.get(artist);
        songs.add(song);
    }
    else {
        byArtist.put(artist, new ArrayList<Song>());
        songs = byArtist.get(artist);
        songs.add(song);
    }
}

If you're looking to minimize code:

for (Song song : mSongs) {
    String artist = song.getArtist();

    if(!byArtist.containsKey(artist))
        byArtist.put(artist, new ArrayList<Song>());

    byArtist.get(artist).add(song);
}

There is one problem with this approach though. HashMaps allow for null values to be explicitly assigned to a key. So in those cases containsKey would return true despite the fact null was stored. So if that's a possibility, you would want to check directly for null.

Adiv Abramson
Adiv Abramson
6,919 Points

Thanks very much. It's amazing how compact you made the code in the final version. I guess that when building the HashMap the programmer should, in order to avoid confusion later on, ensure that only non null values are assigned to the HashMap's keys. Thanks again.