JacksonDunstan.com

Today’s article shows a class that helps clean up your foreach loops when you want to call Add() or Remove() on the List you’re looping over. Normally you’d get an exception, but today’s class works around that problem so your code is less error-prone and easier to read. It also discusses some workarounds you can use even if you don’t use SafeList. Read on to learn how to make your foreach loops less error-prone! UPDATE: SafeList 2.0 is out!

First let’s look at the problem: calling Add() or Remove() on the list you’re looping over with foreach.

foreach (Person person in people)
{
	if (person.Age < 18)
	{
		// This throws an exception because you're modifying the people list
		// that is being looped over with 'foreach'
		people.Remove(person);
	}
}
 
void HandlePersonAdded(Person possibleSibling)
{
	foreach (Person person in people)
	{
		if (person.SiblingName == possibleSibling.Name)
		{
			// This also throws an exception, and for the same reason
			people.Add(possibleSibling);
		}
	}
}

One approach to work around this is to stop using a foreach loop. Unfortunately, that means dropping to the lower-level for loop and dealing with index variables manually. This is error-prone, especially when changing the meaning of the index by adding or removing. Here’s how it works:

// Omit the third statement because we don't always go to the next index
int numPeople = people.Count;
for (var i = 0; i < numPeople;)
{
	Person person = people[i];
	if (person.Age < 18)
	{
		// OK to remove a person since we're not in a 'foreach' loop
		people.Remove(person);
 
		// Update the count of items in the list
		numPeople--;
	}
	else
	{
		// Only go to the next index when we don't remove an item from the list
		i++;
	}
}
 
void HandlePersonAdded(Person possibleSibling)
{
	int numPeople = people.Count;
	for (var i = 0; i < numPeople; ++i)
	{
		Person person = people[i];
		if (person.SiblingName == possibleSibling.Name)
		{
			// This also throws an exception, and for the same reason
			people.Add(possibleSibling);
 
			// Be sure to account for the list size growing
			numPeople++;
		}
	}
}

The code is now littered with statements that have nothing to do with the logic of what we’re trying to accomplish. It’s full of accounting in the form of index variables and list sizes, each needing to be carefully updated at just the right times. The code works now, but it’s error-prone to write and maintain and a lot harder to read.

Another approach to solving the problem is to keep track of the items we wanted to add or remove during the foreach loop and perform the actual Add() and Remove() calls after the loop. Here’s how that looks:

// Make a list of the items to remove
List<Person> toRemove = null;
foreach (Person person in people)
{
	if (person.Age < 18)
	{
		// Create the list if we didn't already
		if (toRemove == null)
		{
			toRemove = new List<Person>();
		}
 
		// Add the item to the list of items to be removed later
		// The item stays in the list we're looping over for now
		toRemove.Add(person);
	}
}
// If there were any items to remove
if (toRemove != null)
{
	// Remove all the items we added to the list from the list we were looping over
	foreach (Person person in toRemove)
	{
		people.Remove(person);
	}
}
 
// Make a list of items to add
List<Person> toAdd = null;
void HandlePersonAdded(Person possibleSibling)
{
	foreach (Person person in people)
	{
		if (person.SiblingName == possibleSibling.Name)
		{
			// Create the list if we haven't before
			if (toAdd == null)
			{
				toAdd = new List<Person>();
			}
 
			// Add the item to add to the temporary list
			// The item remains in the list we're looping over for now
			toAdd.Add(possibleSibling);
		}
	}
	// Add all the items we wanted to add during the loop
	if (toAdd != null)
	{
		people.AddRange(toAdd);
	}
}

This version fixes the problem too, but it’s also cluttered up with bookkeeping code. All that toAdd and toRemove work has nothing to do with the problem we’re trying to solve. It’s just a workaround for a limitation of List and foreach.

Enter the SafeList class. It allows you to iterate as easily as with foreach and simply call Add() or Remove() during the iteration. Here’s how the code looks with SafeList:

people.Iterate(person => {
	if (person.Age < 18)
	{
		people.Remove(person);
	}
});
 
void HandlePersonAdded(Person possibleSibling)
{
	people.Iterate(person => {
		if (person.SiblingName == possibleSibling.Name)
		{
			people.Add(possibleSibling);
		}
	});
}

All of that bookkeeping code—index variables and ancillary lists—is gone and we’re left with just the original logic we wanted to write. The people variable is now a SafeList<Person> rather than a List<Person> and its Iterate() function replaces foreach.

Here’s an expanded example in a Unity script:

using System;
using System.Collections.Generic;
 
using UnityEngine;
 
public class TestScript : MonoBehaviour
{
	class Person
	{
		public string Name;
		public int Age;
		public string SiblingName;
		public override string ToString()
		{
			return string.Format(
				"Person [Name={0}, Age={1}, SiblingName={2}]",
				Name,
				Age,
				SiblingName
			);
		}
	}
	void Start()
	{
		string report = "";
		Action<object> Log = message => report += message + "\n";
 
		var people = new SafeList<Person>();
		people.Add(new Person{Name = "Tom", Age = 22, SiblingName = "Mary"});
		people.Add(new Person{Name = "Bob", Age = 10, SiblingName = "Adam"});
		people.Add(new Person{Name = "Sue", Age = 11, SiblingName = "Herb"});
		people.Add(new Person{Name = "Kat", Age = 24, SiblingName = "Mark"});
 
		Log("Initial people...");
		people.Iterate(person => Log(person));
 
		people.Iterate(person => {
			if (person.Age < 18)
			{
				people.Remove(person);
			}
		});
 
		Log("After removing Age < 18...");
		people.Iterate(person => Log(person));
 
		Person possibleSibling = new Person{
			Name = "Mary", Age = 26, SiblingName = "Tom"
		};
		people.Iterate(person => {
			if (person.SiblingName == possibleSibling.Name)
			{
				people.Add(possibleSibling);
			}
		});
 
		Log("After adding sibling...");
		people.Iterate(person => Log(person));
 
		Debug.Log(report);
	}
}

Here’s the output:

Initial people...
Person [Name=Tom, Age=22, SiblingName=Mary]
Person [Name=Bob, Age=10, SiblingName=Adam]
Person [Name=Sue, Age=11, SiblingName=Herb]
Person [Name=Kat, Age=24, SiblingName=Mark]
After removing Age < 18...
Person [Name=Tom, Age=22, SiblingName=Mary]
Person [Name=Kat, Age=24, SiblingName=Mark]
After adding sibling...
Person [Name=Tom, Age=22, SiblingName=Mary]
Person [Name=Kat, Age=24, SiblingName=Mark]
Person [Name=Mary, Age=26, SiblingName=Tom]

So how does SafeList work? Well, it essentially uses the second workaround version to keep two ancillary lists—toAdd and toRemove—during calls to Iterate(). If you call Add() or Remove() outside of a call to Iterate() the changes will take effect right away.

While the list itself is not immediately modified during the Iterate() call, a Count property is always kept up to date based on the add and remove operations that will take place afterward.

Here’s the source code for SafeList:

using System;
using System.Collections.Generic;
 
/// <summary>
/// A class to queue add and remove operations to a List until after all
/// loops on it have completed.
/// </summary>
/// <author>Jackson Dunstan, http://JacksonDunstan.com/articles/3029</author>
/// <license>MIT</license>
public class SafeList<T>
{
	private List<T> list;
	private List<T> toAdd;
	private List<T> toRemove;
	private int count;
	private int numCurrentIteratingCalls;
 
	public SafeList()
	{
		list = new List<T>();
	}
 
	public SafeList(int capacity)
	{
		list = new List<T>(capacity);
	}
 
	public SafeList(IEnumerable<T> collection)
	{
		list = new List<T>(collection);
		count = list.Count;
	}
 
	public int Count
	{
		get { return count; }
	}
 
	public void Add(T item)
	{
		count++;
		if (numCurrentIteratingCalls > 0)
		{
			if (toAdd == null)
			{
				toAdd = new List<T>();
			}
			toAdd.Add(item);
		}
		else
		{
			list.Add(item);
		}
	}
 
	public void Remove(T item)
	{
		count--;
		if (numCurrentIteratingCalls > 0)
		{
			if (toRemove == null)
			{
				toRemove = new List<T>();
			}
			toRemove.Add(item);
		}
		else
		{
			list.Remove(item);
		}
	}
 
	public void RemoveAll(Predicate<T> match)
	{
		if (toRemove == null)
		{
			toRemove = new List<T>();
		}
		Iterate(item => {
			if (match(item))
			{
				toRemove.Add(item);
				count--;
			}
		});
	}
 
	public void Iterate(Action<T> callback)
	{
		numCurrentIteratingCalls++;
		try
		{
			foreach (var item in list)
			{
				callback(item);
			}
		}
		finally
		{
			numCurrentIteratingCalls--;
			if (numCurrentIteratingCalls == 0)
			{
				if (toAdd != null)
				{
					list.AddRange(toAdd);
					toAdd.Clear();
				}
				if (toRemove != null)
				{
					foreach (var item in toRemove)
					{
						list.Remove(item);
					}
					toRemove.Clear();
				}
			}
		}
	}
 
	public List<T> ToList()
	{
		return new List<T>(list);
	}
}

I hope you find this class useful. It’s not a panacea, but should help clean up some foreach loops in some situations. If you’ve got any ideas to improve it, drop me a line in the comments and let me know!

#1 by Mark Smith on April 22nd, 2015 · Reply

Thanks for this, I have at least a few places where it would be really useful!

For anyone that simply needs to be able to remove items from a List (which is mostly all I ever need, I find I rarely have to add to a List in a loop), I just do the following and it works great:

List items = new List()
{
	"carrot",
	"banana",
	"apple"
};
 
// process in reverse order so that we can do RemoveAt safely
for (int i = items.Count - 1; i >= 0; i--)
{
	string item = items[i];
 
	if (item.Equals("banana"))
	{
		// this works correctly because we are parsing the items in reverse order
		items.RemoveAt(i);
	}
}

It’s pretty easy to remember the syntax, and should be reasonably efficient.

#2 by Mark Smith on April 22nd, 2015 · Reply

Sorry about the lack of code formatting – I did add the “pre” tag, but it didn’t work…and I can’t find a way to edit my comment.
- #3 by jackson on April 22nd, 2015 · Reply
  
  No problem. I added the <pre> tag. Sometimes the comments system gets confused when you have a < or > in your comment, as with your for loop.
  
  And thanks for posting this strategy! I forgot to mention it in the article as a common way to work around the issue. It works really well if you are OK with looping backwards through the list. If you need to go forwards, then you’ll need something more complicated like in the article or to use SafeList.
  
  Another one I forgot to mention is RemoveAll(). Here’s how you’d use it:
  
  items.RemoveAll(item => item == "banana");
  
  That works because it skips the foreach loop entirely. I added RemoveAll() to SafeList also for even safer removing, like a RemoveAll() call from inside some other foreach loop.

#4 by Mark Smith on April 22nd, 2015 · Reply

Hmm, I did add the “pre” tag to my code sample, but it didn’t seem to do anything, and I am unable to edit comments – sorry about that!

#5 by Bernd on July 27th, 2015 · Reply

In your third code block this foreach should probably iterate over toRemove and not people

// If there were any items to remove
if (toRemove != null)
{
// Remove all the items we added to the list from the list we were looping over
foreach (Person person in people)
{
people.Remove(person);
}
}

Otherwise you would just run into the same problem which you are trying to work your way around.

#6 by jackson on July 27th, 2015 · Reply

Thanks for pointing this out! I’ve updated the article to fix the typo.

SafeList: A Class To Eliminate Foreach Errors

Comments