1

I want to create a simple app that is showing recursive group membership, however not users but groups. So if groupA have groupB as a member and groupB have groupC as a member, I want the code to show me all the groups nested till the last one. Here is the sample with what I have:

if (member.Any())
{
    List<string> tempGroup = new List<string>();
    do
    {
        tempGroup.Clear();
        foreach (object nestedGroup in member)
        {
            SearchResult NestedGroupMemberResult = null;
            try
            {
                string NestedgroupMemberSearchFilter = "(&(objectClass=group)(distinguishedName=" + nestedGroup.ToString() + "))";
                string NestedgroupMemberPropertise = "member,distinguishedName";
                NestedGroupMemberResult = dirSearcher(NestedgroupMemberSearchFilter, NestedgroupMemberPropertise);
            }
            catch { }
            if(NestedGroupMemberResult.Properties["member"].Count > 0)
            {
                foreach (object NestedGroupMember in NestedGroupMemberResult.Properties["member"])
                {
                    member.Add(NestedGroupMember.ToString());
                    tempGroup.Add(NestedGroupMember.ToString());
                }
            }
        }
    } while (!tempGroup.Any());
}

listBox1.DataSource = member;

Basically member is a list which contains group members from an input group. Lets say that I initial put GroupA and it have members as GroupB and GroupC and both will be in member list. Now I would like that loop do..while searches for each member of input group using LDAP filter applied, and if it finds any group add it to both member list and tempGroup list, so each time there is a nested group, member list will be updated and at the same time it will be included in foreach loop. If app finds out that there are no more nested groups, the tempGroup list will be empty and loop will stop.

However I'm constantly receiving error "System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'" for

foreach (object nestedGroup in member)

And I guess I cannot modify member list while for loop is iterating through it. I had very similar loop in powershell that works great, but I wanted to have an c# app as I know that it can be much faster in such searches.

varrimatrass
  • 13
  • 1
  • 1
  • 3
  • 1
    You should not use `member.Add()` inside `foreach` loop while still iterating `member` collection contents. Consider read this issue: https://stackoverflow.com/questions/604831/collection-was-modified-enumeration-operation-may-not-execute. – Tetsuya Yamamoto Nov 15 '18 at 09:34
  • 1
    What's the question? – phuzi Nov 15 '18 at 09:34
  • Possible duplicate of [What is the best way to modify a list in a 'foreach' loop?](https://stackoverflow.com/questions/759966/what-is-the-best-way-to-modify-a-list-in-a-foreach-loop) – Jevgeni Geurtsen Nov 15 '18 at 09:34
  • 2
    Possible duplicate of [Collection was modified; enumeration operation may not execute](https://stackoverflow.com/questions/604831/collection-was-modified-enumeration-operation-may-not-execute) – Tetsuya Yamamoto Nov 15 '18 at 09:36
  • The question is how can I iterate through nested groups using foreach loop, but I can see that there are some post about possible duplicate, I will check them and search for an answer. – varrimatrass Nov 15 '18 at 09:52
  • I found my answer here: https://stackoverflow.com/questions/759966/what-is-the-best-way-to-modify-a-list-in-a-foreach-loop switching foreach loop with for is working for me. Thanks @JevgeniGeurtsen – varrimatrass Nov 15 '18 at 11:27
  • @varrimatrass that's not a solution, that covers up the error and causes worse problems. You should *not* modify a collection while iterating it. With `for` you'll end up modifying the wrong items instead of getting an exception – Panagiotis Kanavos Nov 15 '18 at 12:32

4 Answers4

3

Don't modify the collection that you're enumerating with the foreach. Instead you could collect all things that you have to modify:

List<Object> addMembers = new List<Object>();

foreach (object nestedGroup in member)
{
   // ...        
       foreach (object NestedGroupMember in NestedGroupMemberResult.Properties["member"])
        {
            addMembers.Add(NestedGroupMember.ToString());
            //...
        }
    }
    // ...
}

addMembers.ForEach(member.Add);
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
0

Please check this line : member.Add(NestedGroupMember.ToString());

Maybe error occurred when this line executed because you are trying to Add NestedGroupMember.ToString() to member list that are now in a foreach loop, to solve error just create a temporarily variable for this Type just like tempGroup definition and then join them to main member list after foreach ends.

MKH
  • 367
  • 1
  • 4
  • 12
0

There are a few approaches you could take to tackle this problem. The first one, using a temporary list, is mentioned in the answers already. The other approach is to use a for-loop. As I commented, MSDN states the following:

The foreach statement is used to iterate through the collection to get the information that you want, but can not be used to add or remove items from the source collection to avoid unpredictable side effects. If you need to add or remove items from the source collection, use a for loop.

Now some information is missing here for you to make the right, most fitting, choice. If you use a for-loop, and modify the list during enumeration, the items that you add while enumerating will be enumerated too.

So ask yourself the following question: Do the newly added items need to be enumerated too? If so, use a for-loop. If not, you probably want to use the items somewhere else and hence you need to store them somewhere else, for example, in a temporary list.

Jevgeni Geurtsen
  • 3,133
  • 4
  • 17
  • 35
0

You are using foreach, but it doesnt allow you to modify the collection. BUT you can make a reversed for loop, like:

for (var i = list.Length - 1; i >= 0; i-- { var item = list[i]; // TODO }

That must work. Just replace all your foreach loops with collection modification with this reversed for.

misticos
  • 718
  • 5
  • 22