Sab Biuzzino
Sab Biuzzino

Reputation: 49

Treeset doesn't remove object - Java

I'm using Treeset in my project to store an Object class (that i've created early).

I've also implemented build() method that have to add in my treeset the Object and it works great. Now I' ve to implement demolish() method (it has to remove the object specified as parameter), but i've problem: it doesn't remove it:

 private void demolish(int y, String p){
    Iterator iterator = alberobello.iterator();
    while(iterator.hasNext()){
        Edificio edificio = (Edificio) iterator.next();
        if(edificio.getPinodeipalazzi().equals(p) && edificio.getAnno() == y){
            alberobello.remove(edificio);
            dimension --;
            System.out.println("Removed: " + edificio.getPinodeipalazzi()+ " " + edificio.getAnno() + " " +alberobello.size() );
        }
    }
}

This is ALBEROBELLO Declaration

private static TreeSet<Edificio> alberobello;

private static int dimension;

private Skyline(){

    alberobello = new TreeSet<Edificio>();
    dimension = 0;

}

this is EDIFICIO Class

public class Edificio implements Comparable, Iterable{

private int anno;
private String pinodeipalazzi;
private String lato;
private int distanza;
private int base;
private int altezza;

public Edificio(int y, String p, String l, int d, int b, int h){
    this.anno = y;
    this.pinodeipalazzi = p;
    this.lato = l;
    this.distanza = d;
    this.base = b;
    this.altezza = h;
}

public int getAltezza() {
    return altezza;
}

public int getAnno() {
    return anno;
}

public int getBase() {
    return base;
}

public int getDistanza() {
    return distanza;
}

public String getLato() {
    return lato;
}

public String getPinodeipalazzi() {
    return pinodeipalazzi;
}

@Override
public int compareTo(Object o) {
    if((o == null) || this.distanza > ((Edificio)o).distanza)
        return 1;
    /*else if(this.distanza == ((Edificio)o).distanza)
        return 0;*/
    else
        return -1;

}


@Override
public Iterator iterator() {
    return new Iterator() {
        @Override
        public boolean hasNext() {
            return false;
        }

        @Override
        public Object next() {
            return null;
        }
    };
}

@Override
public void forEach(Consumer action) {

}

Upvotes: 2

Views: 1617

Answers (1)

Eran
Eran

Reputation: 393821

Your compareTo method is the source of the problem:

public int compareTo(Object o) {
    if((o == null) || this.distanza > ((Edificio)o).distanza)
        return 1;
    /*else if(this.distanza == ((Edificio)o).distanza)
        return 0;*/
    else
        return -1;

}

Since it never returns 0, the element you are trying to remove from the TreeSet is not found.

I'm not sure why you commented out the part that returns 0, but you should uncomment it:

public int compareTo(Object o) {
    if((o == null) || this.distanza > ((Edificio)o).distanza)
        return 1;
    else if(this.distanza == ((Edificio)o).distanza)
        return 0;
    else
        return -1;
}

P.S. It would be better not to use raw Comparable. Change your class to:

public class Edificio implements Comparable<Edificio>
{
    ...
    @Override
    public int compareTo(Edificio o) {
        if((o == null) || this.distanza > o.distanza)
            return 1;
        else if(this.distanza == o.distanza)
            return 0;
        else
            return -1;

    }
    ...
}

EDIT: Another issue with your code, which was hidden by the failure of remove() to remove the element, is that you are calling alberobello.remove(edificio) while iterating over the Set. That is likely to throw ConcurrentModificationException once you fix your compareTo method. You should simply use the Iterator's remove() method:

    if(edificio.getPinodeipalazzi().equals(p) && edificio.getAnno() == y){
        iterator.remove();
        dimension --;
        System.out.println("Removed: " + edificio.getPinodeipalazzi()+ " " + edificio.getAnno() + " " +alberobello.size() );
    }

Upvotes: 3

Related Questions