2

I have an interesting need for an extension method on the IEumerable interface - the same thing as List.ConvertAll. This has been covered before here and I found one solution here. What I don't like about that solution is he builds a List to hold the converted objects and then returns it. I suspect LINQ wasn't available when he wrote his article, so my implementation is this:

public static class IEnumerableExtension
{
    public static IEnumerable<TOutput> ConvertAll<T, TOutput>(this IEnumerable<T> collection, Func<T, TOutput> converter)
    {
        if (null == converter)
            throw new ArgumentNullException("converter");

        return from item in collection
               select converter(item);
    }
}

What I like better about this is I convert 'on the fly' without having to load the entire list of whatever TOutput's are. Note that I also changed the type of the delegate - from Converter to Func. The compilation is the same but I think it makes my intent clearer - I don't mean for this to be ONLY type conversion.

Which leads me to my question: In my repository layer I have a lot of queries that return lists of ID's - ID's of entities. I used to have several classes that 'converted' these ID's to entities in various ways. With this extension method I am able to boil all that down to code like this:

IEnumerable<Part> GetBlueParts()
{
    IEnumerable<int> keys = GetBluePartKeys();
    return keys.ConvertAll<Part>(PartRepository.Find);
}

where the 'converter' is really the repository's Find-by-ID method. In my case, the 'converter' is potentially doing quite a bit. Does anyone see any problems with this approach?

Community
  • 1
  • 1
n8wrl
  • 19,439
  • 4
  • 63
  • 103

2 Answers2

10

The main issue I see with this approach is it's completely unnecessary.

Your ConvertAll method is nothing different than Enumerable.Select<TSource,TResult>(IEnumerable<TSource>, Func<TSource,TResult>), which is a standard LINQ operator. There's no reason to write an extension method for something that already is in the framework.

You can just do:

IEnumerable<Part> GetBlueParts() 
{ 
    IEnumerable<int> keys = GetBluePartKeys(); 
    return keys.Select<int,Part>(PartRepository.Find); 
} 

Note: your method would require <int,Part> as well to compile, unless PartRepository.Find only works on int, and only returns Part instances. If you want to avoid that, you can probably do:

IEnumerable<Part> GetBlueParts() 
{ 
    IEnumerable<int> keys = GetBluePartKeys(); 
    return keys.Select(i => PartRepository.Find<Part>(i)); // I'm assuming that fits your "Find" syntax...
} 
Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
1

Why not utilize the yield keyword (and only convert each item as it is needed)?

public static class IEnumerableExtension
{
    public static IEnumerable<TOutput> ConvertAll<T, TOutput>
        (this IEnumerable<T> collection, Func<T, TOutput> converter)
    {
        if(null == converter)
            throw new ArgumentNullException("converter");

        foreach(T item in collection)
            yield return converter(item);
    }
}
Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
  • 1
    Technically, using LINQ with Select, which he did previously, streams the results in a very similar manner to this. Granted, I like your version (slightly) better, but it's basically just doing what the Select() method does internally. Both your and the original poster's will only convert as needed, and stream results. – Reed Copsey Jan 08 '10 at 18:22