Tail of list.remove()

When I was checking a code I saw this suspicous line,

list.remove(object)

However, I'm in differenct team hence this is not related to me. Anyway, I informed the developer, who is a Lead, probably senior to me. Since there team is OK with this and it is working for their use case (but not that safe) I have nothing to do but share the knowledge in my blog.

If you google how to remove a value form a list you sure will hit List.remove(E element). Question is, are you using it correctly?

Working scenario

Let's check this out with a String first.


  @Test

  public void testStringListRemove() {

    List<String> list = new ArrayList<>();

    list.add("AB");

    list.add("CD");

    list.add("EF");



    list.remove("CD");



    Assertions.assertEquals(2, list.size());

  }

Test is passing. Does that mean i'm wrong. No, Java String uses a pool hence "CD" is same object. This is completely correct.

Failing Scenario

Now let's see this with an object.


  @Test

  public void testRemovePatient() {

    List<Patient> list = new ArrayList<>();

    list.add(new Patient(1, "NameA"));

    list.add(new Patient(2, "NameB"));

    list.add(new Patient(3, "NameC"));



    list.remove(new Patient(1, "NameA"));



    Assertions.assertEquals(2, list.size());

  }



org.opentest4j.AssertionFailedError: 

Expected :2

Actual   :3

Well, this fails. How about below use case?


  @Test

  public void testStringListRemove() {

    List<String> list = new ArrayList<>();

    list.add("AB");

    list.add("CD");

    list.add("CD");

    list.add("EF");



    list.remove("CD");



    Assertions.assertEquals(2, list.size());

  }



org.opentest4j.AssertionFailedError: 

Expected :2

Actual   :3

Explain Why

First of all let me show you what's in Java DOC.

List.remove(Object o): Removes the first occurrence of the specified element from this list, if it is present (optional operation). If this list does not contain the element, it is unchanged. More formally, removes the element with the lowest index such that (o==null ? e==null : o.equals(e)).

As you can see, default implementation check the object reference as equality, not real equality check. It also mention that it removed the first ocurrence. Why, list can have duplicate values. If you are intending to remove an element, probably you want to remove all.

How does this pass then?


  @Test

  public void testRemovePatient() {

    List<Patient> list = new ArrayList<>();

    Patient nameA = new Patient(1, "NameA");

    list.add(nameA);

    list.add(new Patient(2, "NameB"));

    list.add(new Patient(3, "NameC"));



    list.remove(nameA);



    Assertions.assertEquals(2, list.size());

  }

nameA is the same object. That is why it is passing.

Solution

List removes objects based on the equals(Object obj) method. So,

You should properly implement equals and hashcode.
Use List.removeIf(Predicate p) to remove all.
Modify the logic, instead of removing, adding may solve it.

Remember, your solution may work for a given scenario. However it will fail in future if you are not careful.

Happy coding.

Manju