Am I violating any sound coding principles with this approach?

I am creating a C# service that will be doing some specialized processing of data coming into our ODS (Operational Data Store) DB. To support this, I’m building a DAL that will be used to access our ODS. In my DAL, I’ll have a number of GetItem (get a single item) and GetList (get a list of items) methods with a number of overrides to support a number of combinations of parameters for each method.

I was think that instead of having a number of overrides, I would do the following, using GetList as an example, using an Expression as the only parameter:

/// <summary>
/// Gets a list of Marker entities based on a predicate.
/// </summary>
/// <param name="predicate">The predicate</param>
/// <returns>List of Marker entities</returns>
public List<Entities.Marker> GetList(Expression<Func<MarkerRecord, bool>> predicate)
{
    return this.Database.Marker.Where(predicate).Map();
}

And it would be called like this:

MarkerData markerData = new MarkerData();
var markerList = markerData.GetList(row => row.ReadTime >= new DateTime(2012, 1, 7));

I could even modify this to make it generic to handle any data type. While this solution offers flexibility and saves some coding, something tells me I’m violating one or more best practices, perhaps like separation of concerns by passing code as data. Do you see any problems with this approach?

As you’ve discovered complex DALs are ugly to build because they are just an uninteresting hard to maintain bunch of plumbing 🙁

Fortunately some smart guys have built generics DALs that you can leverage to do almost all the operations like projecting and filtering you want.

When possible they will be processed on the server side which can be a huge gain in performance because you don’t have to pass enormous quantity of data around, just the result you want to further process or display to the user.

Have a look at NHibernate which offers the kind of stuff you’re building plus many more.

In my opinion he is the best on the market but there is some interesting challengers like Entity framework from the fourth version.

And before asking yourself if you’re violating any good practices, principle or dogma, rather ask if it could cause any issue in term of code quality (readability, maintainability, robustness…).

We other geeks are too focused on what is best rather than on what should be done now, it personally took me (a lot of) years, and some experience in front office development, to realize that … but I’m not completely cured. 😉

If the alternative is having overloads like:

public List<Entities.Marker> GetListByReadTime(DateTime minReadTime)

or something like that, then I think your approach is better. Though it’s also going to be more difficult to implement (because you have to parse the Expression), so you should consider if the effort is worth it (it most likely will be).