Suggestions on Working with this Inherited Generic Method

Question

We have inherited a project that is a wrapper around a section of the core business model.

There is one method that takes a generic, finds items matching that type from a member and then returns a list of that type.

public List<T> GetFoos<T>()
{
    List<IFoo> matches = Foos.FindAll(
        f => f.GetType() == typeof(T)
    );

    List<T> resultList = new List<T>();
    foreach (var match in matches)
    {
        resultList.Add((T)obj);
    }
}

Foos can hold the same object cast into various classes in inheritance hierarchy to aggregate totals differently for different UI presentations. There are 20+ different types of descendants that can be returned by GetFoos.

The existing code basically has a big switch statement copied and pasted throughout the code. The code in each section calls GetFoos with its corresponding type.

We are currently refactoring that into one consolidated area, but as we are doing that we are looking at other ways to work with this method.

One thought was to use reflection to pass in the type, and that worked great until we realized the Invoke returned an object, and that it needed to be cast somehow to the List <T>.

Another was to just use the switch statement until 4.0 and then use the dynamic language options.

We welcome any alternate thoughts on how we can work with this method. I have left the code pretty brief, but if you'd like to know any additional details please just ask.

Update

The switch statement was originally using strings and the first pass was moving it into a Presenter with something like this: (No refactoring done on the switch, just where the data went).

// called on an event handler in FooPresenter
// view is the interface for the ASPX page injected into FooPresenter's constructor
// wrapper is the instance of the model wrapper that has the GetFoos method
// selectedFooName is the value of a DropDownList in the Page
//  letting the user say how they want to see the animals
//   its either one big group (Animal)
//   or individual types (Tiger, Lion)
private void LoadFoos(string selectedFooName)
{
    switch (selectedFooName)
    {
        case "Animal":  // abstract base class for all other types
            this.view.SetData(this.wrapper.GetFoos<Animal>(); 

        case "Lion":
            this.view.SetData(this.wrapper.GetFoos<Lion>();
            break;

        case "Tiger":
            this.view.SetData(this.wrapper.GetFoos<Tiger>();    
            break;

        case "Bear":
            this.view.SetData(this.wrapper.GetFoos<Bear>();
            break;    
    }
}

The View implementation (codebehind for an ASPX page)

public void SetData<T>(List<T> data)
{
    // there is a multiview on the page that contains user controls with 
    // grid layouts for the different types

    // there is a control for the common type of "Animal"
    // and other controls for Tiger, Bear, etc

    // the controls contain a 3rd party grid of pain 
    // and the grids' binding event handlers cast the data item 
    // for the row and do some stuff with it specific to that type
}

Our first pass was going to be at least using the Type in the switch statement, or adding an enum.

I played around with using the Strategy Pattern but had to stop when I got to the loading factory returning the List again and realizing I didn't have the type.

Answer 1

It's difficult without seeing the code calling GetFoos()... If you can show more code describing how this is being called, we can suggest how to refactor that.

It sounds like the solution is to make your calling routine a generic routine as well - so that it can avoid the "switch" around the 20 types by just using a single generic type, specified as needed. However, this may not be feasable, but again, without code, it's difficult to know...

That being said, you can refactor GetFoos to be much simpler:

public List<T> GetFoos<T>()
{
    return Foos.OfType<T>().ToList();
}

---

Edit: As Eric Lippert points out, the above code returns any type that is a type of T, but also subclasses of T. Although this is most likely the behavior that would actually be desired, it is different than the original code. If this is not desirable for some reason, you could, instead, use:

public List<T> GetFoos<T>()
{
    Type type = typeof(T);
    return Foos.Where(item => item.GetType() == type).ToList();
}

This will have the same behavior as the original code.

Answer 2

Something like this?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Diagnostics;

namespace SO_2480770 {
    interface IFoo {}
    class MyBase : IFoo {}
    class Bar : MyBase {}

    class Program {               
        IEnumerable<IFoo> Foos { get; set; }

        static void Main(string[] args) {           
            List<MyBase> bases = new List<MyBase>() { new MyBase(), new MyBase() };
            List<Bar> bars = new List<Bar>() { new Bar(), new Bar() };            
            Program p = new Program();
            p.Foos = bases.Concat(bars);            
            var barsFromFoos = p.GetFoos<Bar>();
            var basesFromFoos = p.GetFoos<MyBase>();
            Debug.Assert(barsFromFoos.SequenceEqual(bars));
            Debug.Assert(basesFromFoos.SequenceEqual(bases.Concat(bars)));
            Debug.Assert(!barsFromFoos.SequenceEqual(bases));
            Console.ReadLine();
        }               

        public List<T> GetFoos<T>() where T : IFoo {            
            return Foos.OfType<T>().ToList();
        }
    }    
}

To get rid of big switch statements, you either have to push the generics futher up. I.E. make the method that has the switch statement take a generic type parameter, and keep going until you can't go any futher, if you have to, up the calling chain. When that gets too difficult, think about design patterns such as abstract factory, factory, template methods etc.... It depends on how complex the calling code is.