Is this an example of wrong usage of generics?

Question

Hi,

I wrote this method:

    public IGamePlugin[,] GetTable<T>()
    {
        Type t = typeof(T);

        if (t is IFixedElement)
        {
            return fixedElements;
        }
        else if (t is IFixedTile)
        {
            return fixedTiles;
        }
        else
        {
            throw new NotSupportedException("Type:" + t.ToString() + " is not supported");
        }
    }

And I'm quite unsure if it is not wrong usage of generics. I like it better than using a simple parameter (string or maybe Type) because the syntax is clear on the calling side.

What do you think?

Answer 1

This really should be two separate functions, GetElementsTable and GetTilesTable.

To answer your question, it definitely is abuse of generics the way you've done it. But you're right that using a parameter is also bad.

Answer 2

It is an unusual usage, for sure; it doesn't really use the T, except for typeof(T) . I can see how it may be more useful if you were combining that with generic constraints, but "as is" I'd be tempted to pass a Type instance in as the argument.

BTW, the usage of is is incorrect here; you are testing whether the Type type implements IFixedTile etc, which will never be true (unless you are doing something very unusual); you probably mean to test whether the type represented implements the interface. Perhaps IsAssignableFrom?

Answer 3

It depends, are you going to do anything else with the classes? It seems like a bit of a waste to define interfaces and classes just to make a choice. If you only need to choose between states, use enums:

public enum ElementType
{
    FixedElements, FixedTiles
}

Then you can ask for an ElementType argument.

Answer 4

I agree with Ben Voigt - it should be two different methods.

But actually your code won't work because t is of Type object ant it is not implementing any of your interfaces. Instead you should use t.GetInterface("IFixedElement") != null

Answer 5

It looks like you're returning a specific 2D array depending on the type provided. So the essential parameter you're looking for is not the actual object being passed, but the type. If that's the case, why not simply declare a Dictionary<Type, IGamePlugin[,]> and get the value you need from the dictionary?

I'm thinking like:

// declare this somewhere earlier in the code...
var plugInDictionary = new Dictionary<Type, IGamePlugin[,]>()
{
  {typeof(IFixedElement), fixedElements},
  {typeof(IFixedTile), fixedTiles} 
}
...

// where you would normally call the method GetTable call this instead...
var myGamePlugIn = plugInDictionary[typeof(myObject)];

The advantage of using a dictionary is that it's more dynamic than using an if-block, especially since it accomodates DI more readily than an if-block. What that means is that for instance, you can register the setup above to a config file and actually have to write just a couple of lines of code - the rest is handled by the DI framework. Not that I'm advocating DI for your project, but this was the one example that came to mind for illustrating code flexibility.