0

I have these two lists:

List<image> ImagesByPerimeterId
List<PerimeterTile> ImagesWithMorePerimeters

The context is the following: I want to remove images that contain the id found in the ImagesWithMorePerimeters list from the ImagesByPerimeterId list. The ImagesWithMorePerimeters list has an imageId attribute to compare with the first one.

I have implemented this logic, and it works very well:

foreach(var i in ImagesByPerimeterId) 
{
  foreach(var j in ImagesWithMorePerimeters) 
  {
    if (i.Id == j.ImageId) 
    {
      ImagesByPerimeterId.Remove(i);
    }
  }
}

but I'm looking for a simpler way to compare these lists. Any suggestions?

I tried to use list.Except(), but as the lists are different objects, that did not make it

Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122
  • 2
    there is `List.RemoveAll` ... and `IEnumerable.Any` – Selvin Nov 17 '22 at 14:08
  • \**headdesks in `O(n*m)` time\** – Dai Nov 17 '22 at 14:14
  • 1
    Does this answer your question? [C# comparing two large lists of items by a specific property](https://stackoverflow.com/questions/55138128/c-sharp-comparing-two-large-lists-of-items-by-a-specific-property) – BurnsBA Nov 17 '22 at 14:25
  • There are many different ways to do this. One way is to extract ids you do/don't want, and then filter the collection, e.g., `.Where(x => ids.Contains(x.ImageId))`. Or you can make a custom comparer to pass to `Except`. – BurnsBA Nov 17 '22 at 14:27
  • Define "simpler way"? Your code works pretty well and in fact *is* pretty simple from my point of view. – MakePeaceGreatAgain Nov 17 '22 at 14:30
  • @Selvin That's what I'd expected as well, however OP states it *does* work. – MakePeaceGreatAgain Nov 17 '22 at 14:33
  • 1
    Hehe [...](https://dotnetfiddle.net/6OToCJ) so you think that the real OP problem is this exception? – Selvin Nov 17 '22 at 14:34

1 Answers1

0

You are trying to modify the list while iterating through it, that will cause an exception when the method Remove() is called:

InvalidOperationException: Collection was modified; enumeration operation may not execute.

If you want to do it using iteration do this:

for (var i2 = 0; i2 < ImagesWithMorePerimeters.Count; i2++)
{
    for (var i1 = ImagesByPerimeterId.Count - 1; i1 >= 0; i1--)
    {
        if (ImagesWithMorePerimeters[i2].ImageId == ImagesByPerimeterId[i1].Id)
            ImagesByPerimeterId.RemoveAt(i1);
    }
}

If you want to make it shorter you can do this using Linq:

ImagesByPerimeterId.RemoveAll(image => ImagesWithMorePerimeters.Any(imageMore => image.Id == imageMore.ImageId));

Linq version will be about 5 times slower, here are the benchmark:

|         Method |      Mean |
|--------------- |----------:|
| UsingIteration |  2.314 us |
|      UsingLinq | 10.348 us |
  • Both your versions should be `O(n*m)`, It should be possible to do better by using some kind of lookup table for comparisons, like a HashSet. – JonasH Nov 17 '22 at 15:22