Search code examples
linqextension-methodsabstractionmaintainabilitycyclomatic-complexity

Would you abstract your LINQ queries into extension methods


On my current project we set ourselves some goals for the code metrics "Maintainability Index" and "Cyclometic Complexity". Maintainability Index should be 60 or higher and Cyclometic Complexity 25 or less. We know that the Maintainability Index of 60 and higher is a pretty high one.

We also use a lot of linq to filter/group/select entities. I found out that these linq queries aren't scoring that high on Maintainability Index. Abstracting this queries into extension methods is giving me a higher Maintainability Index, which is good. But in most of the cases the extension methods are not generic anymore because I use them with my Types instead of generic types.

For example the following linq-query vs extension method:

Linq query

List.Where(m => m.BeginTime >= selectionFrom && m.EndTime <= selectionTo)

Extension method:

public static IEnumerable<MyType> FilterBy(this IEnumerable<MyType> source, DateTime selectionFrom, DateTime selectionTo)
{
    return (IEnumerable<MyType>)source.Where(m => m.BeginTime >= selectionFrom && m.EndTime <= selectionTo);
}

List.FilterBy(selectionFrom, selectionTo);

The extension method gives me a Maintainability Index improvement of 6 points, and gives a nice fluent syntax. On the other hand I have to add a static class, it's not generic.

Any ideas on what approach would have your favor? Or maybe have different ideas about how to refactor the linq queries to improve Maintainability Index?


Solution

  • You shouldn't add classes for the sake of metrics. Any metrics are meant to make your code better but following rules blindly, even the best rules, may in fact harm your code.

    I don't think it's a good idea to stick to certain Maintainability and Complexity indexes. I believe they are useful for evaluating old code, i.e. when you inherited a project and need to estimate its complexity. However, it's absurd to extract a method because you haven't scored enough points.

    Only refactor if such refactoring adds value to the code. Such value is a complex human metric inexpressible in numbers, and estimating it is exactly what programming experience is about—finding balance between optimization vs readability vs clean API vs cool code vs simple code vs fast shipping vs generalization vs specification, etc.

    This is the only metric you should follow but it's not always the metric everyone agrees upon...

    As for your example, if the same LINQ query is used over and over, it makes perfect sense to create an EnumerableExtensions in Extensions folder and extract it there. However, if it used once or twice, or is subject to change, verbose query is so much better.

    I also don't understand why you say they are not generic with somewhat negative connotations. You don't need generics everywhere! In fact, when writing extension methods, you should consider the most specific types you can choose as to not pollute other classes' method set. If you want your helper to only work with IEnumerable<MyType>, there is absolutely no shame in declaring an extension method exactly for this IEnumerable<MyType>. By the way, there's redundant casting in your example. Get rid of it.

    And don't forget, tools are stupid. So are we, humans.