CODEWITHSUNDEEP
c#
David Wick
David Wick

Reputation: 23

return not breaking loop (c#)

I'm trying to determine if a user is a member of a group or not in AD. However, the following doesn't seem to be working for some reason...

public bool MemberOf(string sObjectName, string sGroup, bool bIsGroup)
{
    DirectoryEntry dEntry = CreateDirectoryEntry();
    DirectorySearcher dSearcher = new DirectorySearcher(dEntry);
    if (bIsGroup) dSearcher.Filter = "(distinguishedName=" + sObjectName + ")";
    else dSearcher.Filter = "(&(sAMAccountName=" + sObjectName + ")(objectClass=user))";  
    SearchResult sResult = dSearcher.FindOne();
    if (sResult != null)
    {
        foreach (object oGroup in sResult.Properties["MemberOf"])
        {
            if (oGroup.ToString() == sGroup) return true;
            else this.MemberOf(oGroup.ToString(), sGroup, true);
        }
    }
    return false;
}

Another variation: http://users.business.uconn.edu/dwick/work/wtf/6-14-2010%201-15-15%20PM.png

Doesn't work either. This seems like a really dumb question... but shouldn't it break the loop upon "return true;"

Upvotes: 2

Views: 672

Answers (4)

Mark Byers
Mark Byers

Reputation: 839234

It's strange that you're calling this.MemberOf recursively but ignoring the result of it. Perhaps you meant this:

if (oGroup.ToString() == sGroup)
{
    return true;
}
else
{
    bool isMember = this.MemberOf(oGroup.ToString(), sGroup, true);
    if (isMember) { return true; }
}

The reason why your return statment might appear not to exit the loop is because when you call recursively you have loops inside loops inside loops. Check your call stack - you should notice that MemberOf appears multiple time and when you return one of them disappears. By making the above change it will return correctly, unwinding the stack.

Upvotes: 4

Marc Gravell
Marc Gravell

Reputation: 1064244

A return does indeed exit the loop (except for some edge-cases involving anon-methods!). But I'm more worried about the else:

else this.MemberOf(oGroup.ToString(), sGroup, true);

did you mean to do something more like:

else if (this.MemberOf(oGroup.ToString(), sGroup, true)) return true;

?

Note; if so you could tidy this to:

if (oGroup.ToString() == sGroup ||
      this.MemberOf(oGroup.ToString(), sGroup, true)) {
    return true;
}

Upvotes: 1

Timothy S. Van Haren
Timothy S. Van Haren

Reputation: 8966

On this line this.MemberOf(oGroup.ToString(), sGroup, true);, don't you want that to actually be return this.MemberOf(oGroup.ToString(), sGroup, true);? You're calling the same function recursively, and no matter if the function evaluates to true or false inside that recursive loop, the function would always return false.

Upvotes: 1

Mehrdad Afshari
Mehrdad Afshari

Reputation: 422310

Are you sure that line is executing? Maybe the if condition never evaluates to true. Place a breakpoint and check. ... and yes, it should break the loop.

Upvotes: 4

Related Questions