List.Remove does not remove Object

Question

I am implementing the following code to remove an item from a list

// RemoveRole is a member function in a class for Person
// roles is defined as 
// List<PersonOrganisationRoleModel> roles;
// And properly populated prior to this function call

public void RemoveRole(string RoleName)
{
    // I am creating an object that needs to be matched in the list
    PersonOrganisationRoleModel role = new PersonOrganisationRoleModel(OrganisationID, PersonID, RoleName)

    // "role" is now properly constructed, and is matching an exact copy of one of the objects in "roles"
    // the expectation now is the the object "role" must be matched with one of the objects in "roles",
    // and that one be removed
    roles.Remove(role);

}

But the "Remove" function call on the "roles" list does not remove the item in the list that contains the exact same values.

My understanding is that this is supposed to work (it is just the inverse of List.Add

score 4 · Accepted Answer · edited Feb 16 '20 at 07:46

4

The reason is that your role object is not a member of the list and has a different address on memory. By the way if you override Equals method ,like above , you are able to use that. You can also remove via LINQ like this example.

roles.RemoveAll(r => r.OrganisationID == OrganisationID && r.PersonID == PersonID)

This remove all objects in the list by this OrganisationID and PersonID.

edited Feb 16 '20 at 07:46

TanvirArjel

30,049
14
78
114

answered Feb 16 '20 at 07:33

Danial Kalhori

173
5

1

[`List.RemoveAll`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1.removeall?view=netframework-4.8) is not a LINQ method. – Theodor Zoulias Feb 16 '20 at 07:54

Guy · Answer 2 · 2020-02-16T08:28:23.783

3

You need to override Equals() in PersonOrganisationRoleModel class, otherwise the Remove() will compare the objects by reference, which will be false since role is a new object

public override bool Equals(object obj)
{
    if (obj == null)
    {
        return false;
    }

    PersonOrganisationRoleModel p = obj as PersonOrganisationRoleModel;
    if (p == null)
    {
        return false;
    }

    return this.OrganisationID == p.OrganisationID && this.PersonID == p.PersonID;
}

If you are using dictionaries or sets to store this object you should also override GetHashCode()

public override GetHashCode()
{
    return OrganisationID.GetHashCode() ^ PersonID.GetHashCode();
}

edited Feb 16 '20 at 08:28

answered Feb 16 '20 at 07:08

Guy

46,488
10
44
88

1

Overriding `Equals` of reference types is not a good idea, because it messes up the equality semantics. And if you have to do it, you must also override `GetHashCode`. – Theodor Zoulias Feb 16 '20 at 08:00
@TheodorZoulias `GetHashCode` need to be overridden if you use hash based collection like dictionary, but I added it to the answer anyway. I'm not sure why it's not a good idea to override `equals` though, can you explain what you mean by that? – Guy Feb 16 '20 at 08:14
1

If "obj" is not the correct type, then "p" will be null. Solution: check for null after the cast – Hans Keﬆing Feb 16 '20 at 08:15
`PersonID` is a property, not the method `PersonID()`. `GetHashCode()` is incorrect, it seems that you'll need to use `return OrganisationID.GetHashCode() ^ PersonID.GetHashCode();` – Pavel Anikhouski Feb 16 '20 at 08:26
The OP tried to remove a role from their `roles` collection in a way that indicates that they don't have a good grasp of [the differences between reference types and value types](https://stackoverflow.com/questions/5057267/what-is-the-difference-between-a-reference-type-and-value-type-in-c), and how these differences affect the equality semantics of these types. IMHO the solution to this problem is to educate themselves about these differences, and not to learn how to make a reference type behave like a value type. I have used these hacks myself when I was young, and I was bitten later on. – Theodor Zoulias Feb 16 '20 at 08:34

List.Remove does not remove Object

2 Answers2