Can this extension method be improved?
I have the following extension method
public static class ListExtensions
{
public static IEnumerable<T> Search<T>(this ICollection<T> collection, string stringToSearch)
{
foreach (T t in collection)
{
Type k = t.GetType();
PropertyInfo pi = k.GetProperty("Name");
if (pi.GetValue(t, null).Equals(stringToSearch))
{
yield return t;
}
}
}
}
What it does using reflection is it finds the name property and then filters the entry from the collection based on the matching string.
This method is called like
List<FactorClass> listFC = new List<FactorClass>();
listFC.Add(new FactorClass { Name = "BKP", FactorValue="Book to price",IsGlobal =false });
listFC.Add(new FactorClass { Name = "YLD", FactorValue = "Dividend yield", IsGlobal = false });
listFC.Add(new FactorClass { Name = "EPM", FactorValue = "emp", IsGlobal = false });
listFC.Add(new FactorClass { Name = "SE", FactorValue = "something else", IsGlobal = false });
List<FactorClass> listFC1 = listFC.Search("BKP").ToList();
It works fine.
But a closer look at the extension method will show that
Type k = t.GetType();
PropertyInfo pi = k.GetProperty("Name");
actually sits inside a foreach loop which is not really needed. I think we can take it outside the loop.
But how?
Help PLease. (C # 3.0)
a source to share
There are several things you could do — first you could set a generic type constraint on an interface that has a name property. If it can only accept FactorClass, then you don't need a generic type - you can extend it to ICollection<FactorClass>
. If you go for an interface (or with a non-generic version), you can just reference the property and not need reflection. If for some reason this doesn't work, you can do:
var k = typeof(T);
var pi = k.GetProperty("Name");
foreach (T t in collection)
{
if (pi.GetValue(t, null).Equals(stringToSearch))
{
yield return t;
}
}
using an interface that might look like
public static IEnumerable<T> Search<T>(this ICollection<T> collection, string stringToSearch) where T : INameable
{
foreach (T t in collection)
{
if (string.Equals( t.Name, stringToSearch))
{
yield return t;
}
}
}
EDIT : After looking at @Jeff's comment, this is really helpful if you're doing something more complex than just checking the value against one of the properties. He is absolutely correct that using Where
is the best solution for this problem.
a source to share
Using reflection this way is ugly to me.
Are you sure you need a 100% shared "T" and cannot use the base class or interface?
If I were you, I would consider using the .Where<T>(Func<T, Boolean>)
LINQ method instead of writing my own search function.
Usage example:
List<FactorClass> listFC1 = listFC.Where(fc => fc.Name == "BKP").ToList();
a source to share