Posts from  October 2009


10.22.2009

Apply DeMorgan's law refactoring

After reading up on some refactorings after a few hours of refactoring today, I thought it might be nice to share this refactoring with everyone.

I ran into some code today, a simple equality comparison method, except it was comparing about 7 properties, so even worse than this:

public bool Compare(item x, item y)
{
  return !(x.a != y.a || x.b != y.b || x.c != y.c);
}
Even though it doesn't take long to figure out what it is doing, it takes more time to figure out than if we applied DeMorgan's law in what is known as avoiding double negatives refactoring:

public bool Compare(item x, item y)
{
  return x.a == y.a && x.b == y.b && x.c == y.c;
}
Any time you run across code where you negate a set of conditions, do everyone else a favor and invert the conditions. Apply DeMorgan's law to make the code more readable. To freshen up on the rules, visit http://en.wikipedia.org/wiki/De_Morgan's_laws. Saving even 30 seconds of translation every time someone looks at that line of code, becomes a huge savings.

10.5.2009

OrderBy().Descending()

Just wanted to share a quick extension method, it's really simple yes but it's power is in reducing lines of code. If you have ever wanted to apply an order by clause to a collection of items and conditionally do this based on a direction, you know that the only choices available are different methods OrderBy and OrderByDescending. It really is too bad because the internal OrderedEnumerable has a boolean flag for direction that would have been nice as a parameter, but better yet why not make it fluent while we are at it!

Normally we might have to write code like this, assuming I want to order by a particular key in a dictionary field on the item record. (This is just a sample of a nasty selector for ordering that we wouldn't want to be copy & pasting):

if(ascending)
{
  items = items.OrderBy(i => i.DictionaryField.Keys.Contains(key) ? i.DictionaryField[key] : null)
}
else
{
  items = items.OrderByDescending(i => i.DictionaryField.Keys.Contains(key) ? i.DictionaryField[key] : null)
}
The next refactoring might be, which might not be so bad if we could use var instead of Func..., but c# doesn't deal well with implicit functions because of the static typing thing!

Func<Item, object> nastySelector = i => i.DictionaryField.Keys.Contains(key) ? i.DictionaryField[key] : null;
if(ascending)
{
  items = items.OrderBy(nastySelector);
}
else
{
  items = items.OrderByDescending(nastySelector);
}
Next, I can combine the if/else using the ternary operator:

Func<Item, object> nastySelector = i => i.DictionaryField.Keys.Contains(key) ? i.DictionaryField[key] : null;
items = ascending ? items.OrderBy(nastySelector) : items.OrderByDescending(nastySelector);
This is pretty good, but it's not very readable. A chained Descending method (on OrderBy results) would be nice, ridding us of the Func declaration garble and even making our items assignment much more fluent!

ordered = items.OrderBy(i => i.DictionaryField.Keys.Contains(key) ? i.DictionaryField[key] : null);
items = ascending ? ordered : ordered.Descending();
So here is the Descending implementation, this is for Enumerable lists only, if you have an IQueryable collection, cast it to IEnumerable first. This makes use of reflection to set the internal field (descending) on the internal class that you are eternally not supposed to touch :).

public static IOrderedEnumerable<TSource> Descending<TSource>(this IOrderedEnumerable<TSource> source)
{
  var field = source.GetType().GetField("descending", BindingFlags.NonPublic | BindingFlags.Instance);
  if(field == null)
  {
    throw new ArgumentException("Source must be OrderedEnumerable");
  }

field.SetValue(source, true);

return source; }

10.2.2009

ForEach or ForEachCopyIntoNewList?

All of us have desired a ForEach extension method in .Net for a while now, after being spoiled with all the new syntactic sugar with lambdas and linq in c#. We've no doubt all implemented our own, here is the one I use:

public static void ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
  foreach (var item in source)
  {
    action(item);
  }
}
My only issue with this and the foreach loop itself, is that you cannot modify the original collection with your action. There are plenty of cases where we only have a Remove method on a collection and would like to have a RemoveAll. To get around this issue, we can copy items into a new list and iterate over it. With this we can even remove items from the original collection! However, I am now wondering if this should be the default behavior of a ForEach extension method:

public static void ForEachCopyIntoNewList<T>(this IEnumerable<T> source, Action<T> action)
{
  var items = source.ToList();
  items.ForEach(action);
}
I am wondering what everyone thinks, obviously this has implications for delayed execution / lazy loaded scenarios but with that aside, thoughts? I am also looking for a good name to keep this as an alternative extension method but ForEachCopyIntoNewList is rather icky, so if you have a suggestion please let me know.