Am I violating any sound coding principles with this approach?

Question

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?

This is just my opinion, but this kind of expression passing should be handled outside of the DAL in a separate layer. When you introduce this kind of thing into the DAL is can harm maintainability. — emd
– emd, Commented Jun 6, 2013 at 13:12
I don't think you are violating anything with that code, but you do lose something with that approach: explicitness. Where in the overloads variant you know exactly which way your ODS will be called, now you are moving that knowledge/responsibility further up the chain. Is there a specific reason you want to do this? Are there too many overloads? — Erik Schierboom
– Erik Schierboom, Commented Jun 6, 2013 at 13:13
IMO this is a flexible variant of the Specification pattern, and many of the repository patterns on L2SQL or EF use this approach. The only downside is that it does allow the caller to execute fairly arbitrary queries against the database and is can be difficult to 'constrain' (e.g. to prevent too much data being returned, or non-indexed columns from being searched), so I would use internal to your app, but not expose it as an external service etc. Another potential consideration is whether to materialize the result (as you've done), or leave the result as an IQueryable<>, to allow chaining. — StuartLC
– StuartLC, Commented Jun 6, 2013 at 13:22
Have a look here: dofactory.com/Patterns/Patterns.aspx. These are .Net "gang of four" design patterns. Tried and tested, may help you find something to fit your need? Your probably after the creational patterns — Liam
– Liam, Commented Jun 6, 2013 at 13:22

Pragmateek · Accepted Answer · 2013-06-06 13:29:32Z

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. ;)

svick · Accepted Answer · 2013-06-06 13:59:49Z

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

Stack Exchange Network

Am I violating any sound coding principles with this approach?

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Am I violating any sound coding principles with this approach?

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions