2
\$\begingroup\$

Here is some code that reads Active Directory to get a person's manager and parses the result for the managers name. It omits the remaining information. I was just wondering if there might be some more efficient way of doing this instead of the code I am currently using.

var Manager = (string)UserDirectoryData.Properties["manager"].Value;
var StartIndex = Manager.IndexOf("=");
var EndIndex = Manager.IndexOf(",");
StartIndex++;
EndIndex--;
var Length = EndIndex - StartIndex;
Manager = Manager.Substring(StartIndex, Length);

The string returned in the first line of code is in this format:

CN=Joseph Rod,OU=LaptopUser,OU=Users,DC=Company,DC=local

The goal is to just get the name and drop the rest. As the Name length will vary and my RegEX is rusty I could not come up with a working RegEX for this or any other way in C# to pull the name.

Edit:

Buy more efficient I mean faster run times and less resource utilization. This same code will be used in multiple spots in loops of up to 500 iterations (and it will continue to grow as the company does).

\$\endgroup\$
  • \$\begingroup\$ What do you mean by efficient? Faster run-time, less lines, more understandable? var Name = Manager.Split(',')[0].Split('=')[1]is certainly shorter, but is it better? Both solutions require your string to maintain it's format, which may not always be a reasonable assumption. \$\endgroup\$ – Daniel Mar 1 '16 at 17:35
  • \$\begingroup\$ See stackoverflow.com/questions/3833785/… . Seems like there's no built-in way to do this. \$\endgroup\$ – Reinderien Mar 1 '16 at 17:49
  • \$\begingroup\$ @DanielCook Sorry but more efficient I mean faster run times and less resource utilization. This same code will be used in multiple spots in loops of up to 500 iterations (and it will continue to grow as the company does). The string format will not change unless MS changes how AD works and the company upgrade to a version with that change so I am not worried on that. \$\endgroup\$ – Matthew Verstraete Mar 1 '16 at 17:55
  • \$\begingroup\$ @Reinderien Must have missed that in my searches, I will take a look at it, thanks. \$\endgroup\$ – Matthew Verstraete Mar 1 '16 at 17:58
3
\$\begingroup\$

Here's a simple Regex to pull out the CN value:

CN=([^,]*),

You can use this C# code to get the value:

string CN = Regex.Replace(input, @"CN=([^,]*),", "$1");

Here are the important points:

  • I'm assuming you want everything between the "CN=" and the next comma.
  • The Regex replace allows a special string in the last parameter. The "$1" will be replaced with all of the characters in between the parentheses.
  • You can test out the Regex here https://regex101.com/r/dT8lP8/1

In terms of computational efficiency, your solution is most likely more efficient than this Regex solution. You might see an improvement if you replace the second through fifth lines of your solution with this:

var StartIndex = Manager.IndexOf("=") + 1;
var EndIndex = Manager.IndexOf(",", StartIndex) - 1;

I'm using this overload of the IndexOf method that tells the code at what index to start looking for the comma. This starts the search at index 3 ("J") instead of index 0 ("C"). Again, this might run a little faster, but probably not measurably so.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ it doesn't work for DN like this CN=Test'Group\,\"11,OU=ABC,DC=ADOMAIN,DC=LAB \$\endgroup\$ – sagar Feb 14 at 13:09
0
\$\begingroup\$

What happens if your OPs guys screw up and don't enter a manager for somebody? Or your code happens upon the owner, who obviously doesn't have a manager? What happens then?

I'll tell you what happens, you're going to get a NullReferenceException right here.

var StartIndex = Manager.IndexOf("=");

A more robust way of doing this would be to make a second trip to Active Directory to query back the specific user represented by CN=Joseph Rod,OU=LaptopUser,OU=Users,DC=Company,DC=local and let the AD library do the work for you.

If performance becomes a measurable issue, you can implement a cache of all users & their managers that gets initialized when the program starts. AD information shouldn't be changing frequently, so a simple caching strategy would likely be fine.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ If a manager is not set or I hit the owner of the company then var Manager = (string)UserDirectoryData.Properties["manager"].Value; is set to null and when I go to my database later down to get the UserID for that Manager it will return 0 and I will deal with it. \$\endgroup\$ – Matthew Verstraete Mar 2 '16 at 13:38
  • \$\begingroup\$ Have you tried calling IndexOf on a null? \$\endgroup\$ – RubberDuck Mar 2 '16 at 16:38
  • \$\begingroup\$ This is starting to smell like example code. \$\endgroup\$ – RubberDuck Mar 2 '16 at 16:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.