0

I want to remove a List of elements Objects from list base on whatever is inside list2. I used removeAll(). Why does the list not remove those elements that are found in list2?

Element.java

public class Element {

    private String element;

    public Element() {

    }

    public Element(String element) {
        setE(element);
    }


    public String getElement() {
        return element; 
    }

    public void setElement(String element) {
        this.element = element;
    }
}

RemoveAllTest.java

import java.util.ArrayList;
import java.util.List;
public class RemoveAllTest {
    public RemoveAllTest() {
        List<Element> list = new ArrayList<Element>();
        List<Element> list2 = new ArrayList<Element>();

        list.add(new Element("a"));
        list.add(new Element("b"));
        list.add(new Element("c"));

        list2.add(new Element("a"));
        list2.add(new Element("b"));
        list.removeAll(list2);

        for(int i = 0; i < list.size(); i++) {
            System.out.println(list.get(i).getElement());
        }
    }
    public static void main(String[] args) {
        new RemoveAllTest();
    }
}

5 Answers5

4

You need to override equals() and hashCode() methods in Element class.

In this case you must override equals() method. But as a good practice override hashCode() too.

Now let's try to solve your issue. You can use following as equals() and hashCode()

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Element)) return false;

    Element element1 = (Element) o;

    if (element != null ? !element.equals(element1.element) 
                                     : element1.element != null) return false;

    return true;
}

@Override
public int hashCode() {
    return element != null ? element.hashCode() : 0;
}

For more information Why do We need to override the equals and hashCode methods in Java?

Community
  • 1
  • 1
Ruchira Gayan Ranaweera
  • 34,993
  • 17
  • 75
  • 115
  • Oh no. How do I override the methods? sorry I am still learning java – user3871678 Aug 15 '14 at 07:16
  • 1
    For this case you do not need to override hashCode() but it is recommended to override hashCode if you are overriding equals(). http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals%28java.lang.Object%29 – Smith_61 Aug 15 '14 at 07:16
  • @user3871678 If you are using IDE to coding just generate equals() and hashCode() – Ruchira Gayan Ranaweera Aug 15 '14 at 07:17
  • @Smith_61 I believe that the only reason to override hashCode is in case you intend to use these objects as keys in a Map. – Nir Alfasi Aug 15 '14 at 07:37
  • @alfasin that is the main reason why they recommend it. It is meant that Objects that are equal according to equals() they must also have the same hashcode. The default implementation of Object.hashCode() does not guarantee that. Most implementations use the memory location of the Object as it's hashCode(), but we aren't supposed to know that. – Smith_61 Aug 15 '14 at 07:38
2

Because these are not the same elements! every time you're using new operator you're creating a new element!

In order to make two elements that have the same element string equals - you have to override (implement) equals() for example:

@Override
public boolean equals(Object other){
    if (!( other instanceof Element)) {
        return false;
    }
    Element o = (Element)other;
    if (this.element == null) {
        return o.getElement() == null;
    }
    return this.element.equals(o.getElement());
}
Nir Alfasi
  • 53,191
  • 11
  • 86
  • 129
  • 1
    Downvoting without a comment is lame! – Nir Alfasi Aug 15 '14 at 07:17
  • 2
    No idea who added the downvote, but I upvoted because your answer contains the correct information that was needlessly downvoted. – Smith_61 Aug 15 '14 at 07:22
  • This is not properly guarding the NPE. If element is null but o.element is not null the second clause of the or statement will throw an NPE. – Nick Palmer Aug 15 '14 at 07:32
  • @alfasin sorry but you are incorrect. you are using || not &&. false || true evaluates to true. If element == null and o.element != null then the first condition evaluates to false and we evaluate the second condition at which point this.element throws an NPE. If it were false && true I would agree that Java will short circuit the evaluation and avoid the second clause. If the first condition evaluates to true the second evaluation will be skipped with ||. If the first condition is false the second condition will be skipped with &&. Otherwise the second condition must be evaluated. – Nick Palmer Aug 15 '14 at 07:40
2

Your Element class does not implement equals. Because of they use the Object.equals() method which only tests if it is the exact same object. (i.e. that it is == ).

If you implement equals it is good practice to also implement hashCode so that equal objects will hash into the same bucket in hash data structures like Map.

Here is your Element which will not have this problem:

public class Element {

    private String element;

    public Element() {

    }

    public Element(String element) {
        setE(element);
    }


    public String getElement() {
        return element; 
    }

    public void setElement(String element) {
        this.element = element;
    }

    public boolean equals(Object other) {
        if (other instanceof Element) {
            Element otherElement = (Element) other;
            if (null == element) {
                return otherElement.element == null;
            }
            return element.equals(otherElement.element);
        }
        return false;
    }

    public int hashCode() {
        if (null == element) {
            return -7;
        }
        return element.hashCode();
    }
}

Note that we have to be careful about cases where element is null because you are not ensuring that element has a sane value. A "better" element class could avoid this if appropriate by only providing a constructor which takes a String and throwing an IllegalArgumentException if the string passed into the constructor is null, or if the setElement call takes a null value. Like so:

public class Element {

    private String element;

    public Element(String element) {
        setElement(element);
    }

    public String getElement() {
        return element; 
    }

    public void setElement(String element) {
        if (null == element) {
            throw new IllegalArgumentException("Element must not be null.");
        }
        this.element = element;
    }

    public boolean equals(Object other) {
        if (other instanceof Element) {
            Element otherElement = (Element) other;
            return element.equals(otherElement.element);
        }
        return false;
    }

    public int hashCode() {
        return element.hashCode();
    }
}

Finally, note that null == element is better to write than element == null since you can easily forget an equal sign and end up assigning null to element. If you write null = element the compiler will complain and alert you of the dropped character.

Nick Palmer
  • 2,589
  • 1
  • 25
  • 34
0

Use following Element class it should help you.

class Element {

private String element;

public Element() {

}

public Element(String element) {
    setElement(element);
}


public String getElement() {
    return element; 
}

public void setElement(String element) {
    this.element = element;
}

public boolean equals(Object object)
{
    Element now=(Element)object;
    if (object == null || object.getClass() != getClass()) 
    {
        result = false;
    }
    else if(this.getElement().equals(now.getElement()))
    {
        return true;
    }
    else
    {
        return false;
    }
}
public int hashCode() 
 {
    if(element!=null)
      return element.hashCode();
    else
      return 0;
 }
}
Darshan Lila
  • 5,772
  • 2
  • 24
  • 34
0

You could, as an alternative to the other answers, simply reuse the same objects in both lists:

Element a = new Element("a");
Element b = new Element("b");
Element c = new Element("c");

list.add(a);
list.add(b);
list.add(c);

list2.add(a);
list2.add(b);
list.removeAll(list2);

for(int i = 0; i < list.size(); i++) {
    System.out.println(list.get(i).getElement());
}

output:

c