I was reviewing some code the other day that was violating one of our metrics and I saw something like this:

internalclassUtilityClass{

publicstaticstringSomeUtilityMethod(IList<SomeClass> someStuff)

The thing that struck me is this was an internal class and this was the only public method. So the only purpose this class serves is to perform one operation on this closed generic IList.

To me it is more natural and far oop-ier to have that code be part of the collection itself. That may have been the natural instinct of the developer as well but they felt stuck because they are using generics. The more I thought about it using closed generics like this seems problematic.

For example, if I have two lists say:

IList <String> blah;

IList <String> someOtherThings;

And this method:

public staticvoidBlahProcessor(IList<string> blahStrings)

Are those two lists the same thing? Can we treat them the same? Is the list someOtherThings meant to be sent to the method BlahProcessor?

The code as written cannot answer these questions. Someone looking at this kind of code would have to make a determination and, of course, they could make a mistake when doing so.

So what I should be doing instead is creating a new class that inherits from the closed generic that I am using, like so:

publicclassBlahCollection : List<string>

And now my method will look like this instead:

public staticvoidBlahProcessor(BlahCollectionblahStrings)

Now I know that the method is meant to only work on a specific collection of strings. Yes, we did create an extra bit of code but that code is now definitive and purposeful in a way that it previously was not.

I imagine if this was how the original collection had been stated this way in the code then the problematic utility method would never have been created. The developer would have just realized that the collection itself should encaspulate this operation.