views:

187

answers:

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?

+4  A: 

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
+1 for pointing this out...just leaving my answer for those who are still struggling without LINQ.
Justin Niessner
Oh my gawd - Reed you're absolutely right. Shoot - never mind everyone!
n8wrl
A: 

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
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