CodeSOD: Generically Bad

The first two major releases of the .NET Framework, 1.0 and 1.1 were" not good. It's so long ago now that they're easily forgotten, but it's important to remember that a lot of core language features weren't in the framework until .NET 2.0.

Like generics. Generics haven't always been part of the language, but they've been in the language since 2006. The hope would be that, in the course of 13 years, developers would learn to use this feature.

Russell F (recently) has a co-worker who is still working on it.

public static DataTable ClassRegionDToDatatable<POSInvoiceRegionD>(string tableName) where POSInvoiceRegionD : class{ Type classType = typeof(POSInvoiceRegionD); DataTable result = new DataTable(tableName); foreach (PropertyInfo property in classType.GetProperties()) { DataColumn column = new DataColumn(); column.ColumnName = property.Name; column.DataType = property.PropertyType; result.Columns.Add(column); } return result;}public static DataTable ClassRegionFToDatatable<POSInvoiceRegionF>(string tableName) where POSInvoiceRegionF : class{ Type classType = typeof(POSInvoiceRegionF); DataTable result = new DataTable(tableName); foreach (PropertyInfo property in classType.GetProperties()) { DataColumn column = new DataColumn(); column.ColumnName = property.Name; column.DataType = property.PropertyType; result.Columns.Add(column); } return result;}public static DataTable ClassRegionGToDatatable<POSInvoiceRegionG>(string tableName) where POSInvoiceRegionG : class{ Type classType = typeof(POSInvoiceRegionG); DataTable result = new DataTable(tableName); foreach (PropertyInfo property in classType.GetProperties()) { DataColumn column = new DataColumn(); column.ColumnName = property.Name; column.DataType = property.PropertyType; result.Columns.Add(column); } return result;}public static DataTable ClassRegionKToDatatable<POSInvoiceRegionK>(string tableName) where POSInvoiceRegionK : class{ Type classType = typeof(POSInvoiceRegionK); DataTable result = new DataTable(tableName); foreach (PropertyInfo property in classType.GetProperties()) { DataColumn column = new DataColumn(); column.ColumnName = property.Name; column.DataType = property.PropertyType; result.Columns.Add(column); } return result;}

Now, the core idea behind generics is that code which is generic doesn't particularly care about what data-type it's working on. A generic list handles inserts and other list operations without thinking about what it's actually touching.

So, right off the bat, the fact that we have a pile of generic methods which all contain the same code is a simple hint that something's gone terribly wrong.

In this case, each of these methods takes a type parameter (which happens, in this case, to be named just like one of the actual classes we use), and then generates an empty DataTable with the columns configured to match the class. So, for example, you might do:

DataTable d = POSInvoiceRegionUtils.ClassRegionDToDatatable<POSInvoiceRegionD>("the_d");

Of course, because these methods are all generic and accept type parameters, you could just as easily"

DataTable d = POSInvoiceRegionUtils.ClassRegionKToDatatable<POSInvoiceRegionD>("the_d");

Not that such a counterintuitive thing ever happens. By the way, did you notice how these regions are named with letters? And you know how the alphabet has 26 of them? Well, while they're not using all 26 letters, there are a lot more regions than illustrated here, and they all get the same ClassRegion{x}ToDatatable implementation.

So yes, we could boil all of these implementations down into one. Then again, should we? GetProperties is one of .NET's reflection methods, which lets us examine the definition of class objects. Using it isn't wrong, but it's always suspicious. Perhaps we don't need any of this code? Without more information, it's hard to say, but Russell adds:

I'm going to leave aside the question of whether this is something that should be done at all to focus on the fact that it's being done in a really bizarre way.

I'm not sure about "bizarre", but wrong? Definitely. Definitely wrong.

[Advertisement] Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!