It recently came to my attention that exposing generic lists is not something you want to do. Throughout my C# development career, I had thought that using List<T> was acceptable and that it could be used everywhere since it is part of the .NET framework. It turns out, however, that even exposing generic lists ties down your code to a concrete implementation. The primary issue here is that if functionality has to change, you’re stuck with the concrete List type. For example, say you want to add an event to fire when an element is added or removed from the list. List’s Add and Remove methods cannot be overridden, giving you no way to accomplish this task. At that point, you have to change from List to a different type and break your contract with your clients. Another issue is that the List type exposes a lot of properties and methods. Most of the time only a handful of these are actually needed. The rest just give you more opportunities to introduce code smells.

There do exist alternatives, however: Collection<T>, ReadOnlyCollection<T>, and KeyedCollection<T> can be extended via inheritance . Furthermore, they only expose a few methods, leaving a lot of the bloat out. IList<T> is also acceptable, although it still exposes a lot of methods that may not end up being used. However, it does allow you to implement your own versions of adding and removing elements. The concrete List<T> implementation houses Add() and Remove() though: they do not exist in IList<T>.

The question of whether or not exposing List<T> as a method return type, parameter, etc. came to mind when working with our company’s new product Nitriq. One of the many queries that come with the product searches for exposures of List<T>. Not fully grasping why this was a smell, I looked into it. After doing so, I know that my coding habits have changed for good when it comes to using List<T> versus one of the more acceptable alternatives. Not being aware of this smell previously, it shows up a lot in code that I’ve written. It is not the most obvious smell to pick out either since refactoring tools such as ReSharper do not point out this smell (manually scanning for it while developing won’t yield many results either). Fortunately, Nitriq can spot these smells in seconds.

Finding this smell using Nitriq is a very simple task. Let’s have a look at the query “Do not expose generic lists”:

var listOfT = Types.FullNameIs("System.Collections.Generic.List`1").FirstOrDefault();
var results = from type in Typesfrom method in type.Methods
where method.ReturnType == listOfT && !method.IsPrivate && 
    !method.IsProtected && method.Type.IsInCoreAssembly
select new { method.MethodId, method.Name, Type = method.Type.FullName }; 
WarnGreaterThan(results, 0);

The query first acquires an instance of the type List<T> (which is what LIst`1 refers to). It then queries all the methods in all of the types to see if List<T> is being exposed. Finally, it will generate a warning for this query if any results are returned. As you can see from the above sample, Nitriq’s LinqToCode allows you to quickly write up your own custom queries. All of the queries that come with Nitriq are written in this manner and can be easily modified to suit your individual needs.

This post is part of a series exemplifying how Nitriq can assist in the identification of code smells.