patterncsharpMinor
Get users from groups in active directory, disposing of everything
Viewed 0 times
directorygroupsactiveusersgetdisposingeverythingfrom
Problem
I am trying to get active directory groups with their users, all while disposing of all
Since the method is private, and most of the variables only exist in the
Is there anything I can do to reduce the number of
IDisposable assets when they reach the end of their usefulness. The groups and users are passed into a method that extracts certain properties and adds them to ExpandoObjects. This requires a decent number of nested using blocks.Since the method is private, and most of the variables only exist in the
using blocks, I gave the variables very short names. Naming them properly as groupPrincipal or principalSearchResult would involve breaking even more lines to fit the long names.private IEnumerable GetOuGroupsUsersData(
PrincipalContext pc)
{
var data = new List();
using (var gp = new GroupPrincipal(pc))
{
using (var ps = new PrincipalSearcher(gp))
{
using (var psr = ps.FindAll())
{
foreach (var g in psr.GetGroupPrincipals())
{
using (var gpsr = g.GetMembers())
{
data.AddRange(
gpsr.GetUserPrincipals().Select(
u => GenerateData(g, u))
.Cast());
}
}
}
}
}
return data;
}Is there anything I can do to reduce the number of
using blocks? Am I even disposing of everything properly?Solution
-
You can remove braces between
-
Wrapping the
-
I'd suggest you using more appropriate names, maybe by lowercasing just a character of your class/method when you instantiate/call them.
Add this class to your project:
usage:
this is just a more elegant way to do what you asked.
Remember that using a
You can remove braces between
using statements, leaving the last one. That would help you to remove a few of lines, but still not so deeply.-
Wrapping the
IDisposable interface is a good choice to do as an alternative to a dozen of using statements.-
I'd suggest you using more appropriate names, maybe by lowercasing just a character of your class/method when you instantiate/call them.
Add this class to your project:
public class IDisposableList : List, IDisposable
{
public void Dispose()
{
if (this.Count > 0)
{
List exceptions = new List();
foreach (var disposableItem in this)
{
try
{
disposableItem.Dispose();
}
catch (Exception ex)
{
exceptions.Add(ex);
}
}
base.Clear();
if (exceptions.Count > 0)
throw new AggregateException(exceptions);
}
}
public T Add(Func item) where T : IDisposable
{
var disposable = item();
base.Add(disposable);
return disposable;
}
}usage:
var data = new List();
using (var disposables = new IDisposableList())
{
var gp = disposables.Add(() => new GroupPrincipal(pc));
var ps = disposables.Add(() => new PrincipalSearcher(gp));
var psr = disposables.Add(() => ps.FindAll());
foreach (var g in psr.GetGroupPrincipals())
{
var gpsr = disposables.Add(() => g.GetMembers());
data.AddRange(
gpsr.GetUserPrincipals().Select(
u => GenerateData(g, u))
.Cast());
}
}this is just a more elegant way to do what you asked.
Remember that using a
foreach is not as performant as a for loop.Code Snippets
public class IDisposableList : List<IDisposable>, IDisposable
{
public void Dispose()
{
if (this.Count > 0)
{
List<Exception> exceptions = new List<Exception>();
foreach (var disposableItem in this)
{
try
{
disposableItem.Dispose();
}
catch (Exception ex)
{
exceptions.Add(ex);
}
}
base.Clear();
if (exceptions.Count > 0)
throw new AggregateException(exceptions);
}
}
public T Add<T>(Func<T> item) where T : IDisposable
{
var disposable = item();
base.Add(disposable);
return disposable;
}
}var data = new List<ExpandoObject>();
using (var disposables = new IDisposableList())
{
var gp = disposables.Add(() => new GroupPrincipal(pc));
var ps = disposables.Add(() => new PrincipalSearcher(gp));
var psr = disposables.Add(() => ps.FindAll());
foreach (var g in psr.GetGroupPrincipals())
{
var gpsr = disposables.Add(() => g.GetMembers());
data.AddRange(
gpsr.GetUserPrincipals().Select(
u => GenerateData(g, u))
.Cast<ExpandoObject>());
}
}Context
StackExchange Code Review Q#135208, answer score: 5
Revisions (0)
No revisions yet.