1

I have a list of strings that I'm sending to the method. The method takes the list, iterates over it and removes all the empty strings. In the list I'm sending, there are a total of 7 blank strings but the method only seems to be removing 4 of them.

Here's the method:

public List<string> RemoveWhiteSpace(List<string> rawData)
{
    for (int i = 0; i < rawData.Count; i++)
    {
        if (rawData[i].Length == 0)
        {
            rawData.RemoveAt(i);
        }
    }

    return rawData;
}

Here's the list itself copied straight from the console:

  






@0
D=M
@1
D=D-M
@10
D;JGT
@1
D=M
@12
0;JMP
@0
D=M
@2
M=D
@14
0;JMP

This is the output string from the method. It fails to eliminate 3 empty strings. I tested and checked the indices of the strings which aren't being removed, they are indices:4,5 and 6. I also checked the length of these strings and they are in fact 0. I don't understand what the problem is.




@0
D=M
@1
D=D-M
@10
D;JGT
@1
D=M
@12
0;JMP
@0
D=M
@2
M=D
@14
0;JMP
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
SmAsh
  • 19
  • 4

5 Answers5

6

RemoveAt removes the current index, so the index of all elements behind the index you remove will be decremented.

If you want to avoid this, you could start from the end of your list:

for (int i = rawData.Count - 1; i >= 0; i--)
{ 
    if (rawData[i].Length == 0)
    {
        rawData.RemoveAt(i);
    }
}
d4zed
  • 478
  • 2
  • 6
  • 16
4

Why not let .net do the work for you?

list.RemoveAll(item => item.Length == 0);

Or better (note that we remove null strings as well)

list.RemoveAll(item => string.IsNullOrEmpty(item));

Edit: To repair your current loop:

for (int i = 0; i < rawData.Count;) // don't increment here
{
    if (rawData[i].Length == 0)
    {
        rawData.RemoveAt(i);
    }
    else
    {
        i++; // but here 
    }
}
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
  • This is great! I'm new to C# and programming in general so I did not know about this. Is this more efficient than using a loop? – SmAsh Aug 13 '20 at 14:06
  • 1
    @SmAsh: here (with a very small list) the "efficiency" is a question of *nanoseconds*; it's *readability* that's IMHO matters most. I expect the loop to be some ticks ahead – Dmitry Bychenko Aug 13 '20 at 14:09
3

Another option is Linq:

If you want to get rid of just empty lines:

var newData = rawData
    .Where(s => !string.IsNullOrEmpty(s)).ToList();

If you want to get rid of just empty lines and lines with only whitespace:

var newData = rawData
    .Where(s => !string.IsNullOrWhiteSpace(s)).ToList();

Remember that with this code the original collection is left intact so you would need to do further processing with newData

Jason
  • 1,505
  • 5
  • 9
  • Well,`IsNullOrWhiteSpace` may appear *too wide*, e.g. it'll remove `" \t\n \r\n"` string – Dmitry Bychenko Aug 13 '20 at 13:58
  • Yes, the reason that I selected IsNullOrWhiteSpace is that the first entry in his example has two leading spaces, perhaps this was just a copy paste error. Ill update to inform of each option – Jason Aug 13 '20 at 14:01
0

MY guess is that you have some "slots" with consecutive empty strings, and your loop is skipping one of them - here's why.

Suppose you have a list of 4 strings, and (zero-based) indexes 1 and 2 are empty. When i = 1, it detects an empty string and removes it. The problem is that removing the item shifts all remaining items up one index, so the item that was at index 2 is now at index 1. But your loop counter advances to 2, and so the item at index 2 is never checked.

The simple solution is to iterate the list backwards:

for (int i=rawData.Count - 1 ; i >= 0 ; i--)
{
    if (rawData[i].Length==0)
    {
        rawData.RemoveAt(i);
    }
}
D Stanley
  • 149,601
  • 11
  • 178
  • 240
0

You can simply use removeall(predicate) method as a oneliner.

list.RemoveAll(s => string.IsNullOrEmpty(s))

Best regards

RoXTar
  • 164
  • 1
  • 13