Ytukyg

I need to fetch active directory records and insert in SQL database. There are approximately 10,000 records. I have used this code:

List<ADUser> users = new List<ADUser>();



DirectoryEntry entry = new DirectoryEntry("LDAP://xyz.com");

ADUser userToAdd = null;



IList<string> dict = new List<string>();               



DirectorySearcher search = new DirectorySearcher(entry);

search.Filter = "(&(objectClass=user))";

search.PropertiesToLoad.Add("samaccountname");

search.PageSize = 1000;



foreach (SearchResult result in search.FindAll())

{

    DirectoryEntry user = result.GetDirectoryEntry();



    if (user != null && user.Properties["displayName"].Value!=null)

    {

        userToAdd = new ADUser

                    {

                        FullName = Convert.ToString(user.Properties["displayName"].Value),

                        LanId = Convert.ToString(user.Properties["sAMAccountName"].Value)

                    };



        users.Add(userToAdd);

    }

}

How can I optimize the above code in terms of speed and space complexity? Can I use traversal in binary tree as Active Directory structure looks similar to binary tree.

The list returned by DirectorySearcher.FindAll() is just a list. So you can't traverse it any better than you already are.

To optimize this, don't use GetDirectoryEntry(). That is doing two things:

1. Making another network request to AD that is unnecessary since the search can return any attributes you want, and


2. Taking up memory since all those DirectoryEntry objects will stay in memory until you call Dispose() or the GC has time to run (which it won't till your loop finally ends).

First, add displayName to your PropertiesToLoad to make sure that gets returned too. Then you can access each property by using result.Properties[propertyName][0]. Used this way, every property is returned as an array, even if it is a single-value attribute in AD, hence you need the [0].

Also, if your app is staying open after this search completes, make sure you call Dispose() on the SearchResultCollection that comes out of FindAll(). In the documentation for FindAll(), the "Remarks" section indicates you could have a memory leak if you don't. Or you can just put it in a using block:

DirectorySearcher search = new DirectorySearcher(entry);

search.Filter = "(&(objectClass=user))";

search.PropertiesToLoad.Add("sAMAccountName");

search.PropertiesToLoad.Add("displayName");

search.PageSize = 1000;

using (SearchResultCollection results = search.FindAll()) {

    foreach (SearchResult result in results) {

        if (result.Properties["displayName"][0] != null) {

            userToAdd = new ADUser {

                FullName = (string) result.Properties["displayName"][0],

                LanId = (string) result.Properties["sAMAccountName"][0]

            };



            users.Add(userToAdd);

        }

    }

}