why do i get an unsupported operation exception when i want to remove an object from a treeset

Question

i have this 3 class :

first one

private List<Domino> pile = new ArrayList<Domino>();    
    
    public DominoPile(List<Domino> list) {
        this.pile = list;
            
    }
    public List<Domino> getList(){
        return List.copyOf(pile);
    }
    public void getOutLastOne() {
        pile.remove(this.getLast(0));
    }
    public Domino getLast(int nbr) {
        return nbr  == 0 || nbr > this.getSize() ? pile.get(pile.size() - 1) : pile.get(pile.size() - 1 - nbr);
    }
    public int getSize() {
        return pile.size();
    }
    
}

second one

public class KingdominoGameFactory {
    
    private List<Player> pl;
    private List<Domino> dom;
    private int selected;
    private DominoPile pile;
    private int nbrPl;
    
    
    public KingdominoGameFactory(List<Player> p, List<Domino> d) {
        // TODO Auto-generated constructor stub
        this.setPl(List.copyOf(p));
        this.setDom(List.copyOf(d));
    }


    public List<Player> getPl() {
        return pl;
    }


    public void setPl(List<Player> pl) {
        this.pl = pl;
    }


    public List<Domino> getDom() {
        return dom;
    }


    public void setDom(List<Domino> dom) {
        this.dom = dom;
    }


    public int getSelected() {
        return selected;
    }


    public void setSelected(int selected) {
        this.selected = selected;
    }


    public int getNbrPl() {
        return nbrPl;
    }


    public void setNbrPl() {
        if(selected == 0) this.nbrPl = 2;
        else if(selected == 1) this.nbrPl = 3;
        else this.nbrPl = 4;
    }

    public DominoPile getPile() {
        return pile;
    }


    private void setPile(DominoPile pile) {
        this.pile = pile;
    }
    public void nbrDomFinal() {
        if(this.getNbrPl() == 2) this.setPile(new DominoPile(this.getDom().subList(0, 24)));
        else if(this.getNbrPl() == 3) this.setPile(new DominoPile(this.getDom().subList(0, 36)));
        else this.setPile(new DominoPile(this.getDom()));
    }
    public void nbrPlFinal() {
        if(this.getNbrPl() == 2) this.setPl(this.getPl().subList(0, 2));
        else if(this.getNbrPl() == 3) this.setPl(this.getPl().subList(0, 3));
        else this.setPl(this.getPl());
    }

third one

public class Game {

    private List<Player> players;
    private DominoPile pile;
    private DrawLine actual = new DrawLine(new TreeSet<Domino>());
    private final int nbrDraw;
    
    public Game(KingdominoGameFactory kg){
        this.nbrDraw = kg.getNbrPl() == 3 ? 3 : 4;
        this.pile = new DominoPile(List.copyOf(kg.getPile().getList()));
        this.players = this.getListPlayers();
    }
    private void addActual(){
        actual.add(pile.getLast(0));
    }
    public int getNbrDraw() {
        return nbrDraw;
    }
    public List <Player> getListPlayers(){
        return players;
    }
    public void setDrawActual() {
        actual.clear();
        for(int i = 0; i < nbrDraw; ++i) {
            this.addActual();
            pile.getOutLastOne();
        }
    }
    public DrawLine getActual() {
        return actual;
    }
    
    public DominoPile getPile() {
        return pile;
    }
    
}

then i have this test made by me

    @BeforeEach
    void setup() {
        List<Domino> liste = new ArrayList<Domino>();
        liste.add(new Domino(1, new Tile(Terrain.CASTLE, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(2, new Tile(Terrain.CASTLE, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(3, new Tile(Terrain.CASTLE, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(4, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(5, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(6, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(7, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(8, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(9, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(10, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(11, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(12, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(13, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(14, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(15, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(16, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(17, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(18, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(19, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(20, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(21, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(22, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(23, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(24, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        liste.add(new Domino(25, new Tile(Terrain.EMPTY, 2), new Tile(Terrain.CASTLE, 1)));
        List<Player> liste2 = new ArrayList<Player>();
        liste2.add(new Player("Jeff", "366"));
        liste2.add(new Player("Jeff", "366"));
        liste2.add(new Player("Jeff", "366"));
        liste2.add(new Player("Jeff", "366"));
        
        kg = new KingdominoGameFactory(liste2, liste);
        kg.setSelected(0);
        kg.setNbrPl();
        kg.nbrDomFinal();
        kg.nbrPlFinal();
        
        game = new Game(kg);
    }


    @Test
    void setActual() {
        game.setDrawActual();
        assertEquals(game.getActual().getSize(), 4);
    }

then i have this error

java.lang.UnsupportedOperationException
    at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:72)
    at java.base/java.util.ImmutableCollections$AbstractImmutableCollection.remove(ImmutableCollections.java:79)
    at kingdomino.domains.DominoPile.getOutLastOne(DominoPile.java:23)
    at kingdomino.domains.Game.setDrawActual(Game.java:35)
    at kingdomino.domains.GameTest.setActual(GameTest.java:87)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)

i do not know why can't i remove from Game class because the tests for DominoPile are all successfull, it is the only test that has problems, i do not know how i can fix this problem i have tried to change my collection type, i have also tried to change my method "getOutLastOne"

Answer 1

List.copyOf

This makes an immutable (i.e.: Read Only) copy of whatever you pass to it. There is no further point in making copies of this (and, in fact, I believe if you pass an already-immutable list in there it just returns it verbatim; no point making copies of things that cannot be modified).

When creating a DominoPile object, you pass the result of List.copyOf to its constructor, meaning, the pile field in DominoPile is now one of these immutable lists. A while later your code invokes getOutLastOne which modifies (mutates) the list referenced by the pile field, which doesn't work, because it cannot be modified. As the exception states.

That's the problem. There are 3 different strategies you can use to fix the problem. In broad strokes:

1. Document in the constructor of DominoPile that the passed-in list is taken verbatim and will be modified as is, meaning: It must not be immutable (it's now the duty of the caller to read the docs and do what it says), and any effects caused on the list by DominoPile (such as removing the last one) echo through to the list used to create the DominoPile object in the first place.

Once you've done that (basically, all you need to do is, add some javadoc), then the caller (where you currently have new DominoPile(List.copyOf(kg....))) needs to stop passing immutable list. This is easy enough - replace List.copyOf(X) with new ArrayList<>(X) - that also makes a copy, but a mutable one.

2. Alternatively, you can also choose a slightly different design: Have DominoPile treat the provided list merely as a template, and thus have your DominoPile constructor make a new (mutable!) list that is a copy of it. In other words, replace this.pile = pile, both in the constructor and the set method, with this.pile = new ArrayList<>(pile) instead. But I kinda dislike the name setPile if that's what happens.

3. DominoPile becomes immutable, all set methods need to go away, all data structures become immutable, and a method like .getOutLastOne doesn't actually remove anything (it can't - everything cannot be modified), instead it makes an entirely new DominoPile object that is a clone of itself, except with one domino removed. Now the fact that your lists are immutable is, if anything, a benefit. This is... over-engineering things by quite a large margin, I think.

The options are listed in order of how I'd do it - so, unless you have a pressing reason why the first option sounds bad to you, I'd just go with the first, here. Based on your API design (such as having a set method, meaning you do not appear to particularly desire immutability).