This is my personal blog. The views expressed on these pages are mine alone and not those of my employer.

Thursday, 30 August 2012

LINQ or readability?

We all know that cyclomatic complexity is directly correlated to bugs in your code and a great way to remedy this is by reducing the lines of code you write.

I recently stumbled across this article by Scott Hansleman which considered a number of scenarios where LINQ has the ability to improve our collective code-bases, through reducing lines.

To take an extract from his blog:
I visit a lot of customers and look at a lot of code. I also worked with a number of large production code bases in my previous jobs and I see a lot of ifs, fors and switches. I see loops inside of loops with ifs inside them, all doing various transformations of data from one form to another....I'd much rather write these one line operations than the loop and if above...(which are) simpler, terser, and less error prone than loops of loops.
However, like Scott points out, it is equally important to emphasise you should not cloud the water in an attempt to condense lines.  Readability is also king.

Here is an example that ReSharper recently suggested be condensed.

Code with a foreach loop:
decimal? total = 0;

foreach (var halfHour in halfHours)
{

    total += Calculate(data, halfHour.DateAndTime.Date, new
 List<string> {halfHour.HalfHourNumber});

}

return total;

Code using LINQ:
return halfHours.Aggregate<HalfHour, decimal?>(0, (current, halfHour) => 
current + Calculate(data, halfHour.DateAndTime.Date, new 
List<string> {halfHour.HalfHourNumber}));

This is certainly one for judgement, but I'd go with the most readable any day. So what do you think?


Update:

I've become aware that infact this article is relating to a lambda expression rather than LINQ itself (which can't be used interchangeably), and therefore the post isn't factually correct in that regard. I'll be describing in a future post what each of these mean and the differences between the two, as I hadn't been aware of the distinct difference myself. The core point of the post remains however.

5 comments:

  1. I agree, you should go with whatever form makes it the most readable. However, the issue here is that the refactoring suggested by Resharper is suboptimal. Here is a better form:

    return halfHours.Select(halfHour => Calculate(data, halfHour.DateAndTime.Date, new List { halfHour.HalfHourNumber }))
    .Sum();

    To make either form of the code more readable, I would suggest changing the name of Calculate to describe what it's actually calculating, and changing the last parameter of the Calculate method to use params.

    ReplyDelete
    Replies
    1. I forgot about Sum's overload; the projection is unnecessary.

      return halfHours.Sum(halfHour => Calculate(data, halfHour.DateAndTime.Date, new List { halfHour.HalfHourNumber }));

      With the last parameter as params:

      return halfHours.Sum(halfHour => Calculate(data, halfHour.DateAndTime.Date, halfHour.HalfHourNumber));

      With the Calculate method moved to halfHour (likely belongs there as it's entangled with data from halfHour) and with Calculate + data renamed to something more meaningful (making it up since I can't tell from the article):

      return halfHours.Sum(h => h.CalculateDifferential(times));

      Delete
  2. Thanks for that Chris, I'd certainly pick your version over the foreach or ReSharper's suggestion.

    I guess the real point here is that LINQ certainly isn't the silver bullet in improving readability (as some articles would suggest) and that it is equally susceptible to obfuscation as traditional methods. This is especially true when relying on tools to generate the 'optimal' code for you.

    Over time this will improve (as more developers use LINQ and tools improve) and we'll begin to see more of positive benefits such as efficient execution along with more readable code.

    ReplyDelete
  3. I want to to thank you for this wonderful read!!
    I absolutely enjoyed every little bit of it.
    I've got you book marked to look at new stuff you post…

    Here is my website: google keyword tool

    ReplyDelete
  4. oh these bugs in codes(( but must say that with your hints I'm having now less of them! thanks! By the way, I'm coding fro virtual data rooms for mergers and acquisitions

    ReplyDelete