removing specific string values from a List

Question

I have a List of string values and and some of the values contains xxx or XXX in front.

xxxRed
xxxYellow
xxxxCareful with that axe Eugene!
xxxxxxdedicum aceasta frumoasa Melodia
xxxxLeaders
xxxxWorking Around - titles
XXXXXNothing To Fear
xxxxAvoiding standards
xxxFirst Aid

List<string> lstTitles = new List<string>();

This is what I have tried

for (int i=0; i < lstTitles.Count; i++)
            {
                string title = lstTitles[i].ToLower().Trim();
                if (title[0] == 'x')
                {
                    lstTitles.Remove(lstTitles[i]);

                }
            }

Problem I have is that only some of the values are removed but not all of them.

Is there perhaps a better way of removing these values?

I4V · Accepted Answer · 2013-06-06T11:21:12.797

10

Use RemoveAll method

lstTitles.RemoveAll(s => s[0] == 'x' || s[0] == 'X');

and you may want to use StartsWith instead of comparing first char.

lstTitles.RemoveAll(s => s.StartsWith("x",StringComparison.InvariantCultureIgnoreCase));

edited Jun 06 '13 at 11:21

answered Jun 06 '13 at 11:15

I4V

34,891
6
67
79

2

`|| s[0] == 'X'`... also, would fail if a string item is null or empty. – Rotem Jun 06 '13 at 11:16
Does not handle the case, and OP mentionned wants to remove items starting with 'xxx', not 'x' ;) – Manitra Andriamitondra Jun 06 '13 at 11:19
@l4V, actually, OP's code is wrong, because, it would remove 'Xavier' from the list. And i'm sure he doesn't want that – Manitra Andriamitondra Jun 06 '13 at 11:22

CodeCaster · Answer 2 · 2013-06-06T11:52:56.243

3

Problem I have is that Only some of the values are removed but not all of them.

Because you're skipping items. When you call Remove(), the next item will be at index i, but you'll increase i in the next loop.

It can be solved by iterating over a copy of the list, and removing unwanted items in the original:

foreach (var item in lstTitles.ToList())
{
    if (item.StartsWith("x", StringComparison.InvariantCultureIgnoreCase))
    {
        lstTitles.Remove(item);
    }
}

Though this involves creating a copy of the list, which isn't really useful, as well as calling Remove() which itself is far from performant.

So you could invert your for-loop, to remove the last items first which doesn't change the indexing for unprocessed items:

for (int i = lstTitles.Count - 1; i > 0; i--)
{
    if (lstTitles[i].StartsWith("x", StringComparison.InvariantCultureIgnoreCase))
    {
        lstTitles.RemoveAt(i);
    }
}

But as @I4V points out, all of this logic already is in List<T>.RemoveAll(), which is nicer to read and probably optimized for some edge cases, so there's little use to hand-code it again.

edited Jun 06 '13 at 11:52

answered Jun 06 '13 at 11:14

CodeCaster

147,647
23
218
272

This will throw an error because the collection has been modified while in a foreach loop. – Sean Airey Jun 06 '13 at 11:15
1

@Sean no it doesn't, because I call `.ToList()`. – CodeCaster Jun 06 '13 at 11:15
2

Oh, sorry, missed that, my bad =] – Sean Airey Jun 06 '13 at 11:16
So in every iteration you're creating a new collection from the old. That's really inefficient. – Tim Schmelter Jun 06 '13 at 11:23
@TimSchmelter that is [not how `foreach()` works](http://stackoverflow.com/questions/13250561/does-foreach-execute-the-query-only-once). – CodeCaster Jun 06 '13 at 11:24
@CodeCaster: Yes i know. So there's no reason to use a `foreach` in this case. Instead you should use a backwards `for`-loop or `RemoveAll`(best). – Tim Schmelter Jun 06 '13 at 11:27
@TimSchmelter then don't say "in every iteration". :-) But agreed, updated. – CodeCaster Jun 06 '13 at 11:31

score 2 · Answer 3 · answered Jun 06 '13 at 11:16

That's because your skipping values.

Suppose your list contains ['xVal1', 'xVal2', 'val3', 'xVal4', 'val5']. At first your i is 0, and you look at list[0], which is 'xVal1', so you remove it.

Now your list contains ['xVal2', 'val3', 'xVal4', 'val5'], and your i is 1. So you look at list[1] which is 'val3'. You ignored xVal2 !

You can start at the back of the list and go to the front, although you will still have a potential bug in case there are identical values you remove.

A shorter way would be to use LINQ:

var newList = lstTitles.Where(title=>!title.StartsWith('xxx'))

score 2 · Answer 4 · answered Jun 06 '13 at 11:16

Instead of ToLower you should use the overload of StartsWith which allows to pass a StringComparison.OrdinalIgnoreCase.

Then use List.RemoveAll which is the most readable, most efficient and shortest approach:

lstTitles.RemoveAll(s => s.TrimStart().StartsWith("x", StringComparison.OrdinalIgnoreCase));

Demo

score 2 · Answer 5 · answered Jun 06 '13 at 11:17

2

I think, you'd better just create a new list this way

list = list
    .Where(i => ! i.StartsWith("xxx", StringComparison.InvariantCultureIgnoreCase))
    .ToList();

It would have a O(n) complexity whereas, trying to remove then 1 by 1 would be in O(n^2).

answered Jun 06 '13 at 11:17

Manitra Andriamitondra

1,249
1
15
21

Why would looping over a list once have a complexity of > O(n)? – CodeCaster Jun 06 '13 at 11:39
I was refering to @CodeCaster's code which uses list.Remove(string) which is by it self in O(n). So if you loop through the list, and for some items, you use list.Remove(string), you could have a combined complexity of O(n^2). – Manitra Andriamitondra Jun 06 '13 at 11:49

score 1 · Answer 6 · answered Jun 06 '13 at 11:24

1

This could work also :

list.RemoveAll(i => i.StartsWith("xxx", StringComparison.InvariantCultureIgnoreCase));

Handles all cases and without a second list.

answered Jun 06 '13 at 11:24

Manitra Andriamitondra

1,249
1
15
21

removing specific string values from a List

6 Answers6