CodeSOD: Highly Paid Entities

Years ago, Samuel's company brought in some Highly Paid Consultants. The HPCs brought with them a binder full of best practices, a dedicated Agile coach, and a slick project plan that promised to accomplish everything the company needed in half the time budgeted.

One of their "best practices" was the advice that "ORMs are considered harmful," and while the existing codebase already made liberal use of .NET's Entity Framework, their new code would be "optimized".

Those optimizations must have thrown off the project timeline- they burned through their "aggressive" project plan's budget, then the original time-and-cost estimates, and then lingered for months trying to finish their tasks. Eventually, they moved on to the next contract, and Samuel was handed their code and told to fix the bugs and performance issues.

This was HR software, so among other things, it would track employees' "career plans". One example query the software needed to answer was to find the one plan that was the greatest number of "challenge units" below a certain threshold.

The "harmful" Entity Framework/LINQ way of answering this question would be code akin to this:

 var selectedLevel = _careerPlanRepository.GetAll() .Where(c => c.ChallengeHistoricalUnits <= units) .OrderByDescending(t => t.ChallengeHistoricalUnits) .FirstOrDefault();

EF/LINQ do a little bit of magic here: the chain of method calls are used to generate an SQL statement, send the SQL statement to the database, and then capture the results. So, for example, in this case, GetAll doesn't really get all the records- it gets all the records which pass the Where function. And because we explicitly tag a FirstOrDefault on the end, it knows it only needs to fetch one row, and thus doesn't fetch data we don't want.

Perhaps it's the magic or the (in this case) efficiency which is harmful. Here's how the HPCs cut down on their use of Entity Framework:

var plans = _careerPlanRepository.GetAll().OrderByDescending(t => t.ChallengeHistoricalUnits).ToList();foreach (var item in plans){if (item.ChallengeHistoricalUnits <= units){selectedLevel = item;break;}}

You'll note that they got rid of the Where call, so their non-harmful approach is to fetch every row from the table into a list. Then they look at every item in that (sorted) list and find the first one which meets the threshold.

The HPCs left behind their binder of best practices. It's seen a lot of use around the office, as a doorstop, paperweight, or monitor stand. Beyond that, though, Samuel has quite consciously done the "harmful" thing and started using EF and LINQ to streamline his queries.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!