Geeks With Blogs
Path Notes of a Kodefu Master blog

in·ane (ĭn-ān') adj. in·an·er, in·an·est
One that lacks sense or substance.

The American Heritage® Dictionary of the English Language, Fourth Edition
Copyright © 2009 by Houghton Mifflin Company.
Published by Houghton Mifflin Company. All rights reserved.

One of the first signs that code needs to be refactored is that it looks complex. Complex solutions are rather easy to come by, but are far less maintainable than their harder to write, easier to read brethren. Like any well-crafted sculpture, it didn’t like start that way, and it requires work to craft that beautiful piece of code. But sometimes you come across code for which there is no reason for the complexity. It appears that someone just wanted to type extra characters throughout their code.

A while back, I analyzed another developer’s code that used a lot of generics. Since nothing was being returned from many of these methods, I decided to look at the signature to discover why the generics were necessary. Here’s one of them.

public static void PrintReport<T>(T report)
{
    WriteEvent(report.ToString());
    IReport r = report as ClientReport;
    if (r != null)
    {
        //Do stuff
    }
    r = report as EmployeeReport;
    if (r != null)
    {
        //Do stuff
    }
}

Looking at this, you may notice there is absolutely no reason for the generic declaration. There are no constraints on it, so it is the same thing as declaring the parameter as an object. Everwhere this was used, it was calling PrintReport<ClientReport>(report) and so on. I pointed this out to the developer, and said it should be refactored because a) the generic adds no value, and b) there are object oriented solutions that are much better than static methods and casting. Of course, this is what I got back.

public static void PrintReport<T>(T report) where T : IReport
{
    WriteEvent(report.ToString());
    IReport r = report as ClientReport;
    if (r != null)
    {
        //Do stuff
    }
    r = report as EmployeeReport;
    if (r != null)
    {
        //Do stuff
    }
}

There’s now a constraint! A constraint that has no meaning other than preventing someone from printing an object that isn’t an IReport. Of course, the same thing could be accomplished by making the parameter be an IReport rather than a T. I then showed the developer how the generic adds absolutely no value (I think I saw a grimace as I cut all of those <xReport>s out of the myriad of classes making that call). I then explained how when I see things like casting in a method like that, it makes me consider that perhaps that behavior belongs on the class, perhaps called by a method defined on the interface.

public static void PrintReport(IReport report)
{
    WriteEvent(report.ToString());
    report.DoStuff();
}

For future reference, inane usage of generics involves using a generic on a method, not involving a parameterized class, where 1) the return type isn’t the generic type, and 2) the constraint isn’t new() or class.

If someone can think of a situation where this is actually useful, post a comment.

Also, another quick lesson. The developer did place PrintReport<ClientReport> in many places, but did you know that this wasn’t even necessary? With generic methods, you get generic inference. The compiler will determine the type for you. This  is very handy with LINQ. Without generic inference, you would have to write ugly methods like strings.Where<string>(s => s.StartsWith(“s”)).

Note: Cross posted from KodefuGuru.
Permalink
Posted on Tuesday, September 29, 2009 6:05 PM | Back to top


Comments on this post: Inane Generic Usage

# re: Inane Generic Usage
Requesting Gravatar...
Good job! THANKS! You guys do a great website, and have some great contents. Keep up the good work.
best regards,
thailand hotels
Left by Thai Restaurant on Mar 08, 2010 2:24 AM

Your comment:
 (will show your gravatar)


Copyright © Chris Eargle | Powered by: GeeksWithBlogs.net