Remove object from generic list by id

Question

I have a domain class like this:

public class DomainClass
{
  public virtual string name{get;set;}
  public virtual IList<Note> Notes{get;set;}
}

How would I go about removing an item from the IList<Note>? I would be able to do it if it was a List but it has to be an IList as I am using Nhibernate for my persistance layer.

Ideally I wanted a method like this in my domain class:

public virtual void RemoveNote(int id)
{
   //remove the note from the list here

   List<Note> notes = (List<Note>)Notes

   notes.RemoveAll(delegate (Note note)
   {
       return (note.Id = id)
   });
}

But I can't cast the IList as a List. Is there a more elegant way round this?

Interesting problem, you don't want to cast to the concrete type because you don't know it before. I guess if you loop on all elements would work but would be slow. I don't know the answer, I wonder if selecting the node to delete with a LINQ query would help then every concrete class used at runtime will execute the linq query faster or slower depending on the list type, sorted, ordered or not... — Davide Piras, Jun 25 '11 at 14:01
Do you have notes with the same ID? If not you might want to use a `IDictionary` instead — Magnus, Jun 25 '11 at 14:03

Gabe · Accepted Answer · 2014-05-20T14:27:17.190

40

You could filter out the items you don't want and create a new list with only the items you do want:

public virtual void RemoveNote(int id)
{
   //remove the note from the list here

   Notes = Notes.Where(note => note.Id != id).ToList();
}

edited May 20 '14 at 14:27

answered Jun 25 '11 at 14:30

Gabe

84,912
12
139
238

Nice - i was going at it from completely a different angle. How efficient will this be though? – gdp Jun 25 '11 at 14:45
There shouldn't be much of a difference. You're having to traverse the whole list anyway. – luqui Jun 25 '11 at 20:34

Vercas · Answer 2 · 2011-06-25T14:17:46.067

17

Edit2: This method doesn't require casting to a List!

foreach (var n in Notes.Where(note => note.Id == id).ToArray()) Notes.Remove(n);

or...

Notes.Remove(Notes.Where(note => note.Id == id).First());

The first one is the best.
The second one will throw an exception if no notes have that id.

Edit: Thanks to Magnus and rsbarro for showing my mistake.

edited Jun 25 '11 at 14:17

answered Jun 25 '11 at 14:00

Vercas

8,931
15
66
106

4

Removing items from a list while iterating it, not gonna work. Do a `.ToList()` – Magnus Jun 25 '11 at 14:05
2

@Magnus is right, the first one will throw a `InvalidOperationException: Collection was modified; enumeration operation may not execute.`. – rsbarro Jun 25 '11 at 14:07
1

This could be simplified by writing: `Notes.Remove(Notes.First(note => note.Id == id));` – Jay Rainey Sep 10 '18 at 12:56

score 2 · Answer 3 · edited May 23 '17 at 12:16

You can either code it manually. The naive implementation is O(n*k) with n the number of items in the list, and k the number of items you want to remove. If you want to just remove a single item it is fast.

But if you want to remove many items then the native implementation becomes O(n^2) for many IList<T> implementations(including List<T>, no idea how NHibernate's list behaves) and you need to write a bit more code to get a O(n) RemoveAll implementation.

One possible implementation from an old answer: List, not lose the reference

The trick with this implementation is that in moves the kept items to the beginning of the list in O(n). Then it keeps removing the last item of the list(which is usually O(1) since no elements need to move), so the truncation becomes O(n) total. This means the whole algorithm is O(n).

score 2 · Answer 4 · answered Jun 25 '11 at 14:57

If you can change the datastructure I would suggest using a Dictionary. Than you can go with:

public class DomainClass
{
  public virtual string name{get;set;}
  public virtual IDictionary<int, Note> Notes {get; set;}

  //Helper property to get the notes in the dictionary
  public IEnumerable<Note> AllNotes
  {
    get
    {
      return notes.Select (n => n.Value);
    }
  }

  public virtual void RemoveNote(int id)
  {
     Notes.Remove(id);
  }

}

If ID is not unique use IDictionary<int, IList<Note>> instead.

sll · Answer 5 · 2011-06-29T12:59:08.903

1

Please consider, that in some cases better to avoid public virtuals, use template method pattern in such a way:

 public void Load(IExecutionContext context) 
 { 
      // Can safely set properties, call methods, add events, etc...
      this.Load(context);            
      // Can safely set properties, call methods, add events, etc. 
 }

 protected virtual void Load(IExecutionContext context) 
 {
 }

edited Jun 29 '11 at 12:59

answered Jun 29 '11 at 12:24

sll

61,540
22
104
156

I'd like to see some kind of justification for avoiding *public virtuals*. – spender Jun 29 '11 at 12:31
@spender, by overriding a virtual method you can corrupt base class behaviour by skipping a base.Method() call. This approach also well known as NVI idiom. – sll Jun 29 '11 at 12:36
@sll Calling the base method in an overridden method may or may not be appropriate depending on what the base method does e.g. if you overrode ToString(), in most cases there would be no value to calling base.ToString() in your overridden method. – Ben Robinson Jun 29 '11 at 12:47
@Ben Robinson, I must agree, you are exactly right. I should midify my post by adding "in some cases consider". Thanks for a point! – sll Jun 29 '11 at 12:58
@Ben Robinson, regarding overriding of the ToString(), lets image PersonBase base class with properties Id and Name, so its ToString() will return "Id=.. Name...", and all nested classes should just concatenate base.ToString() + own ToString() implementation – sll Jun 29 '11 at 14:22

score 0 · Answer 6 · answered Jun 25 '11 at 14:58

You can receive an array of items for removing. Than remove them from list in loop. Look at this sample:

IList<int> list = new List<int> { 1, 2, 3, 4, 5, 1, 3, 5 };

var valuesToRemove = list.Where(i => i == 1).ToArray();

foreach (var item in valuesToRemove)
{
    list.Remove(item);
}

score 0 · Answer 7 · answered Dec 20 '22 at 21:17

0

Just List.RemoveAt("ID");

or better..

try
{
     List.RemoveAt("ID");
}
catch
{
     Console.WriteLine("Out of bounds.");
}

answered Dec 20 '22 at 21:17

score 0 · Answer 8 · answered Mar 06 '23 at 09:31

My solution, first make a Hashset list for the items to be deleted, then call RemoveAll items which contains the id of the HashSet:

HashSet<Guid> vListIdToDelete = new HashSet<Guid>();//TODO fill here
vOtherList.RemoveAll(p => vListIdToDelete.Contains(p.ID));

Remove object from generic list by id

8 Answers8

Linked