Casting problem in C# generic method

Question

I'm having some trouble with a generic method I'm writing. It has the following signature;

public static ThingCollection<T> GetThings<T>(...) where T : Thing

There are several classes; ThingA, ThingB and ThingC that inherit from Thing; and I want to be able to have code something like this in the method.

var things = new ThingCollection<T>();

if (typeof(T) == typeof(Thing))
  foreach (var item in someCollection)
    things.Add((T)new Thing(...));
else if (typeof(T) == typeof(ThingA))
  foreach (var item in someCollection)
    things.Add((T)new ThingA(...));
else if (typeof(T) == typeof(ThingB))
  foreach (var item in someCollection)
    things.Add((T)new ThingB(...));
else if (typeof(T) == typeof(ThingC))
  foreach (var item in someCollection)
    things.Add((T)new ThingC(...));
else
  throw new Exception("Cannot return things of type " + typeof(T).ToString());

return things;

The problem is that I get a best overloaded method match has invalid arguments error if I don't cast the new objects. Adding the T casts as shown above is fine for the new Thing() but reports Cannot convert type 'ThingA' to 'T' for the other new calls. Intellisense indicates that T is a Thing but I don't understand why I can't cast the other objects to Thing, as they inherit from it.

Perhaps this is not the right way to be doing what I'm trying to do. Am I on the right track? Perhaps missing some small nuance, or should I be doing something else entirely?

Answer 1

If they use a common interface (IThing) you should be able to cast to that.

Answer 2

Primarly I think, this code snippet has bad design. If you add "ThingD" class, you need change in another part of code, for clear behaviour. You should use something like:

public static ThingCollection<T> GetThings<T>(...) where T : Thing, new()
...
...
T item = new T();
item.Something = Whatever();

Or you can implement the "ICloneable" interface int Thing class.

Answer 3

The code violates Liskov substitution principle, as it tries to test the type of T before using it.

To avoid this you could use dictionary/strategy combination or visitor pattern.

If T is ThingB the cast (T)ThingA is invalid, so the code is actually wrong.

Answer 4

I don't get what you are trying to do with that code.

If you want to create a Collection of Things where you could add any type of class derived from Thing, ThingCollection should not have a Typename: it's supposed to be a collection for concrete types.

E.g, implementing A ThingCollection this way:

public class ThingCollection : List<Thing> {}

now you can do

ThingCollection tc = new ThingCollection();
tc.Add(new ThingA());
tc.Add(new ThingB());
tc.Add(new ThingC());

Assuming of course that ThingA, ThingB and ThingC inherits from Thing.

Or maybe you want to filter derived types of Things with the GetThings() i.e. you want that a call to GetThings() returns a ThingCollection.