Search code examples
javaarrayslinked-listconcurrentmodification

Exception in thread "main" java.util.ConcurrentModificationException, Not sure why


this 'playlist' class should be able to skip replay and go back a song in a playlist which is a linkedlist of songs, which is another class, however I get the error Exception in thread "main" java.util.ConcurrentModificationException here is the playlist class which I have written: (thank you in advanced)

 package com.sulay;

import java.util.LinkedList;
import java.util.ListIterator;

public class Playlist {

private LinkedList<Song> songs;
private ListIterator<Song> listIterator;
private boolean goingForward = true;

public Playlist() {
    this.songs = new LinkedList<>();
    this.listIterator = this.songs.listIterator();
}

public LinkedList<Song> getSongs() {
    return this.songs;
}

public void addSong(Song song) {
    if(!checkSong(song)) {
        this.songs.add(song);
        System.out.println("Song " + song.getTitle() + " added");
    } else {
        System.out.println("Song " + song.getTitle() + " already exists");
    }
}

public boolean checkSong(Song song) {
    for (Song currentSong : songs) {
        if (currentSong.equals(song)) {
            return true;
        }
    }
    return false;
}

public void skipSong() {
    if (listIterator.hasNext()) {
        
        if (!goingForward) {
            listIterator.next();
            goingForward = true;
        }
        System.out.println(listIterator.next() + " now playing");
    } else {
        System.out.println("No next song");
    }
}

public void previousSong() {
    if (listIterator.hasNext()) {
        
        if (goingForward) {
            listIterator.next();
            goingForward = false;
        }

        System.out.println(listIterator.previous() + " now playing");
    } else {
        System.out.println("No previous song");
    }
}

public void replaySong() {
    if (goingForward){
        goingForward = false;
        System.out.println("Playing " + listIterator.previous());
    } else if (!goingForward) {
        goingForward = true;
        listIterator.next();
        System.out.println("Playing " + listIterator.previous());
    }
}

public void displaySongs() {
    for (Song currentSong : songs) {
        System.out.println("Name: " + currentSong + " Album: " + currentSong.getAlbum());
    }
}

}

Solution

  • If you allow additions of new songs, the state of the listIterator's state is not synchronized with the list. So it now may throw a ConcurrentModificationException at any point

    The best approach is not to expose or use the list after initialization. So after you start playing the playlist, you cannot add or remove from it. Or you may just use an arraylist, which is RandomAccess,i.e it supports constant-time get methods and a index variable. This will not need any listiterator.

    Remember that you ARE exposing the mutable list through the getSongs method. To make it safe, make it's return value List<Song> and return this

    return new ArrayList<>(songs);