Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been working with C# for quite some time but relatively new to the concepts of lambdas and Linq. I was working with a Linq To SQL example and trying to write a generic solution for executing a search against a collection of entities (eg. list of Customer objects) where the search parameter will be specified by passing a partially filled object of the entity class itself. I was doing this just to ensure that a person does not have to go on specifying different overloads for different searches based on the entity class attributes and trying to write a generic solution which will cater to any entity class.

Hence I used reflection and an array of predicates to successively apply the Where clauses to the collection.

Client App

using (CustomerManager oCustomerManager = new CustomerManager())
{
   IEnumerable<Customer> customers = oCustomerManager.Load();

   Customer oSearchCustomer = new Customer();
   oSearchCustomer.City = "London";
   oSearchCustomer.ContactName = "Thomas Hardy";

   IEnumerable<Customer> customerList = oCustomerManager.Search(oSearchCustomer);

   foreach (Customer customer in customerList)
   {
      Console.WriteLine(customer.ToString());
   }
}

Manager Class

public IEnumerable<Customer> Search(Customer searchObject)
{
    IEnumerable<Customer> customers = DataContext.Customers;
    List<Func<Customer, bool>> result = 

    DataContext.Customers.GenerateFilterClause<Customer>(searchObject);

    foreach (var item in result)
    {
        customers = customers.Where(item);
    }

    return customers;
 }

Extension Method Utility Class

public static class UtilityExtensions
{
   public static List<Func<T, bool>> GenerateFilterClause<T>(this IEnumerable<T> 
          collection, T searchEntity)
   {
        List<Func<T, bool>> whereFilterList = new List<Func<T, bool>>();
        Func<T, bool> predicate = null;

        var propertyList = searchEntity.GetType().GetProperties();

        foreach (PropertyInfo p in propertyList)
        {
            if (p.GetCustomAttributes(false).OfType<ColumnAttribute>().Count() > 0)
            {
               string propName = p.Name;
               var searchVal = 

               searchEntity.GetType().GetProperty(propName).GetValue(searchEntity,null);
               if (searchVal != null)
               {
                  predicate = new Func<T, bool>(entity => propertyWhereClause(entity, 
                         searchEntity, propName, searchVal));
                  whereFilterList.Add(predicate);
               }
            }
         }

         return whereFilterList;
     }

      private static bool propertyWhereClause<T>(T obj, T searchEntity, string    propertyName, object searchVal)
     {
           return obj.GetType().GetProperty(propertyName).GetValue(obj, 
                                   null).Equals(searchVal);
      }
  }

While the solution is working, I have two specific questions:

  • What are the drawbacks of this solution? I was hoping to have inputs regarding performance and cleaner way to rewrite the code.
  • Can anyone suggest a more efficient and better solution? That would help me to understand the proper way to utilize the power of lambdas and Linq.
share|improve this question

3 Answers 3

How much did you look into LINQ to SQL? Unless you have a reason not to expose the DataContext, you can just perform queries on it without using the 'search object'.

from c in DataContext.Customers
where c.City == "London" && c.ContactName == "Thomas Hardy"
select c

I haven't used LINQ to SQL myself yet, so I can't really tell much about the best practices and whether or not exposing DataContext is a good idea. Probably it's best to use an existing ORM framework. DataObjects.NET looks really nice.

A nice ORM overview can be found on ORMbattle.

share|improve this answer

I don't fully understand what you're trying to accomplish, but there's a lot of code here that doesn't make sense.

Client app

  • You create a customers object and never use it. Why?
  • You create a temporary customerList object only to iterate over it. Why?
  • WriteLine(object) already calls ToString implicitly.

Manager class and the rest

  • You're reinventing the wheel here. This is exactly what LINQ-to-SQL does.
share|improve this answer

I won't be able to answer the performance part as I'm not too clued up about LinqToSQL, as for clean code, you could rewrite your utility entirely by taking advantage of Lambdas and anonymous types

public static IEnumerable<Func<T, bool>> GenerateFilterClause<T>(this IEnumerable<T> collection, T searchEntity)
{
    var filters = searchEntity.GetType()
                              .GetProperties()
                              .Where(property => !property.GetCustomAttributes<ColumnAttribute>(false).Any())
                              .Select(property => new
                              {
                                  Name = property.Name,
                                  Value = property.GetValue(searchEntity, null)
                              })
                              .Where(property => property.Value != null)
                              .Select(property => new Func<T, bool>(arg => arg.GetType()
                                                                              .GetProperty(property.Name)
                                                                              .GetValue(arg, null)
                                                                              .Equals(property.Value)));

    return filters;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.