Removing items from a list while iterating through it?

Archived Forums Questions & Answers
1

So what I’m doing is iterating through a list which is displaying a set of boxes representing each item in the list. Each of those boxes representing the items has a button in it that lets the user delete that item from the list (any item at any place in the list). Of course, this is throwing an exception since I’m removing an item from a list while iterating it.

The only answer I’ve seen so far involves iterating backwards through the list and then deleting within the for loop. I’ve tried this solution and it wasn’t preventing the exception when I remove an item, so here I am trying to figure out what I can do to get around this error.

I can post the old code if someone wants to see what I tried to do with the backward iteration solution.

	using UnityEngine;
    using System.Collections;
     
    public class SceneManager : MonoBehaviour {
		string name;
		string gender;
		string type;
		public bool showCreateActor = false;
		public ArrayList actors;
		 
		void Start () {
			actors = new ArrayList();
			name = string.Empty;
			gender = string.Empty;
			type = string.Empty;
		}
		 
		public void removeActor(ActorGUI self){
			actors.Remove(self);
		}
     
	void ActorWindowSide(int windowID) {
		
		if(GUILayout.Button ("Add Actor", GUILayout.Width (100),GUILayout.Height (30))){
			showCreateActor = !showCreateActor;
		}
		GUILayout.BeginVertical();
		//the loop in question
		foreach(ActorGUI actor in actors){
			actors*.Draw();*

  •  }*

  •  GUILayout.EndVertical();*
  • }*

public void createNewActor(int windowID)
{

  •  GUI.Label (new Rect(5f,30f,75f,20f), "Name: ");*

  •  GUI.Label (new Rect(5f,55f,75f,20f), "Gender: ");*

  •  name = GUI.TextField(new Rect (85f,30f,100f,25f), name, 25);*

  •  gender = GUI.TextField(new Rect (85f,55f,100f,25f), gender, 10);*

  •  if(GUI.Button (new Rect(85f, 260f, 110f, 30f), "Create Actor")){*

  •  	ActorGUI a = gameObject.AddComponent("ActorGUI") as ActorGUI;*

  •  	a.setActorNumber (actors.Count);*

  •  	a.setActorName (name);*

  •  	a.setActorGender (gender);*

  •  	actors.Add(a);*

  •  	showCreateActor = !showCreateActor;*

  •  	name = string.Empty;*

  •  	gender = string.Empty;*

  •  }*

  •  if(GUI.Button (new Rect(205f, 260f, 110f, 30f), "Cancel")){*

  •  	showCreateActor = !showCreateActor;*

  •  	name = string.Empty;*

  •  	gender = string.Empty;*

  •  }*

  • }*
    }

using UnityEngine;
using System;
using System.Collections;

public class ActorGUI : MonoBehaviour {

  •  public int actorNumber;*

  •  public string actorName;*

  •  public string actorGender;*

  •  private SceneManager mySceneManager;*

  •  private ActorGUI self;*

  •  void Start () {*

  •  	mySceneManager = gameObject.GetComponent("SceneManager") as SceneManager;*

  •  }*

  •  public void setActorNumber(int i) {*

  •  	actorNumber = i;*

  •  }*

  •  public void setActorName(string name){*

  •  	actorName = name;*

  •  }*

  •  public void setActorGender(string gender){*

  •  	actorGender = gender;*

  •  }*

  •  public void setSelf(ActorGUI me){*

  •  	self = me;*

  •  }*

  •  public void Draw(){*

  •  	GUILayout.BeginVertical("box");*

  •  	GUILayout.BeginHorizontal();*

  •  	if(GUILayout.Button (actorName,GUILayout.Height(30))){*

  •  	}*

  •  	if(GUILayout.Button ("X", GUILayout.Width(30),GUILayout.Height(25))){*

  •  		mySceneManager.removeActor(self);*

  •  	}*

  •  	GUILayout.EndHorizontal();*

  •  	GUILayout.EndVertical();*

  •  }*

}

the easiest way is to GO BACKWARDS. it's a a basic general programming trick.

 –

3 Answers

3
3

You cannot remove an item from a List that you are iterating over with foreach. You have three choices:

  • a backwards running for…next loop and RemoveAt
  • make a copy of the list your are iterating (with Linq.ToList())
  • make a list of the things to remove and then RemoveRange them after the list.

Now you've updated your code it would appear that adding the actor to a "removals" list would be the most efficient method (Linq would be easier to write but it would cause garbage collection). Why are you defining a variable called self which is exactly the same as this?

 –

Also don't use ArrayList - use a generic List< ActorGUI> which is faster and typesafe.

 –

I've tried both List<> and ArrayList, this just happens to be the version w/ ArrayList. I didn't use this because I forgot I could, thanks for the reminder. The Linq.ToList() method sounds better (or at least faster), I'm not that concerned about garbage collection at the moment, I'll probably end up giving both a try. Thanks.

 –

So that would look like using System.Linq; ... foreach(ActorGUI actor in actors.ToList())

 –
2

I don’t see the loop in your code. I checked three times.

But you had the right idea, you have to iterate through it backward. You can remove items as you go.

You cannot remove items from a for-each - it has to be a for(int variable ... type loop.

If you want to post the code that’s failing while you try to backward iterate, I’ll look again.

Oh hmm, I had it all grouped together, I guess it got missed in the copy/paste. Fixing now...

 –
4

for(int i =0;i<“yourlist”.Count;i++)
{
if(“yourlist” == “condition to remove”)
{
“yourlist”.removeAt(i);
i–;
}
}