This post originated from an RSS feed registered with .NET Buzz by Eric Gunnerson. Original Post: Don't write code like this... Feed Title: Eric Gunnerson's C# Compendium Feed URL: /msdnerror.htm?aspxerrorpath=/ericgu/Rss.aspx Feed Description: Eric comments on C#, programming and dotnet in general, and the aerodynamic characteristics of the red-nosed flying squirrel of the Lesser Antilles Latest .NET Buzz Posts Latest .NET Buzz Posts by Eric Gunnerson Latest Posts From Eric Gunnerson's C# Compendium

I've just finished doing a security review of my MSDN columns, and as part of that, I needed to add some security to an add-in architecture, so the add-in architecture won't run antisocial code. The approach taken by .NET security is reminiscent of ACLs, which means (at least to me) that it's confusing and I need to read the code 5 or 6 times before I understand it. Along the way, I got a chunk of code from somebody on the .NET security team, which I'll reproduce here:

private NamedPermissionSet FindNamedPermissionSet(string name)
{
	IEnumerator policyEnumerator = SecurityManager.PolicyHierarchy();

	while (policyEnumerator.MoveNext())
	{
		PolicyLevel currentLevel = (PolicyLevel)policyEnumerator.Current;

		if (currentLevel.Label == "Machine")
		{
			IList namedPermissions = currentLevel.NamedPermissionSets;
			IEnumerator namedPermission = namedPermissions.GetEnumerator();

			while (namedPermission.MoveNext())
			{
				if (((NamedPermissionSet)namedPermission.Current).Name == name)
				{
					return ((NamedPermissionSet)namedPermission.Current);
				}
			}
		}
	}
	return null;
}
    

Ugly, isn't it?

The root of the problem is that PolicyHierarchy returns an IEnumerator instead of an object that supports IENumerable.  This means that I can't use foreach on it, and have to write the traversal myself. This decision also means that, without a collection returned, there's no place to hang a useful method like "LookupLabel()" on, so all users are stuck writing the search code themselves (or encapsulating it off into a separate method).

PolicyLevel.NamedPermissionSets uses a different approach, that of returning an IList, which is better than an enumerator, as I could use foreach on the IList, but the person who wrote this sample didn't because they didn't know about foreach. If they had, they would have done the right thing.  

We need to not do this.