Question

Is there a way to make this code more efficient ?

if (includeRows != null && includeRows.Count > 0)
{
    for (int i = aList.Count - 1; i >= 0; i--)
    {
        if (!includeRows.Exists(j => j == (i + 1)))
        {
            aList.RemoveAt(i);
            includeRows.Remove(i + 1);
        }
    }
}

This is what i did , the aList contains objects not integers , so need the index of the object in the list.Not sure if the includeRows.Remove() would make it less or more efficient, includeRows was just changed to a HashSet.

for (int i = aList.Count - 1; i >= 0; i--) {
                    if (!includeRows.Contains(i + 1) )
                    {
                        aList.RemoveAt(i);
                       // includeRows.Remove(i + 1);
                    }
 }
Was it helpful?

Solution

Building on p.s.w.g's answer, I would do:

HashSet<int> includeRowsFaster = new HashSet<int>(includeRows);
aList.RemoveAll(i => !includeRowsFaster.Contains(i + 1));

for most efficient performance and readibility. Looking for an element in includeRows is an O(n) complexity operation. You can reduce it significantly to O(log(n)) by using a hashset instead of a vector (array or list) implementation.

See this for a discussion on Hashset vs. List performance : https://stackoverflow.com/a/10762995/390330

OTHER TIPS

Here's an easy way with Linq's Intersect method:

aList = aList.Intersect(includeRows).ToList();

But for better performance you can use RemoveAll instead

aList.RemoveAll(i => !includeRows.Exists(j => j == (i + 1));
aList = aList.Intersect(includeRows).ToList<int>();
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top