Get instance of a class that inherits from abstract class and have specific value in a property

Question

I'm working on a factory to create concrete instances based in two criteria:

1) The class must inherit from an specific abstract class

2) The class must have an specific value in an overridden property

My code looks like this:

public abstract class CommandBase
{
    public abstract string Prefix { get; }
}

public class PaintCommand : CommandBase
{
    public override string Prefix { get; } = "P";
}

public class WalkCommand : CommandBase
{
    public override string Prefix { get; } = "W";
}

class Program
{
    static void Main(string[] args)
    {
        var paintCommand = GetInstance("P");
        var walkCommand = GetInstance("W");  

        Console.ReadKey();
    }

    static CommandBase GetInstance(string prefix)
    {
        try
        {            
            var currentAssembly = Assembly.GetExecutingAssembly();
            var concreteType = currentAssembly.GetTypes().Where(t => t.IsSubclassOf(typeof(CommandBase)) &&
                                                                     !t.IsAbstract &&
                                                                     t.GetProperty("Prefix").GetValue(t).ToString() == prefix).SingleOrDefault();

            if (concreteType == null)
                throw new InvalidCastException($"No concrete type found for command: {prefix}");

            return (CommandBase)Activator.CreateInstance(concreteType);
        }
        catch (Exception ex)
        {            
            return default(CommandBase);
        }
    }
}

I'm getting the error:

{System.Reflection.TargetException: Object does not match target type. at System.Reflection.RuntimeMethodInfo.CheckConsistency(Object target) at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

Answer 1

A cleaner way is to define your own attribute to store the prefix.

[AttributeUsage(AttributeTargets.Class)]
public class CommandAttribute : Attribute
{
    public String Prefix { get; set; }

    public CommandAttribute(string commandPrefix)
    {
        Prefix = commandPrefix;
    }
}

Then use them like so:

[CommandAttribute("P")]
public class PaintCommand : CommandBase
{}

[CommandAttribute("W")]
public class WalkCommand : CommandBase
{}

In reflection:

static CommandBase GetInstance(string prefix)
{       
    var currentAssembly = Assembly.GetExecutingAssembly();
    var concreteType = currentAssembly.GetTypes().Where(commandClass => commandClass.IsDefined(typeof(CommandAttribute), false) && commandClass.GetCustomAttribute<CommandAttribute>().Prefix == prefix).FirstOrDefault();

    if (concreteType == null)
        throw new InvalidCastException($"No concrete type found for command: {prefix}");

    return (CommandBase)Activator.CreateInstance(concreteType);
}

Answer 2

As spender alluded to in his comment, the reason you are getting this specific error is this line:

t.GetProperty("Prefix").GetValue(t)

Here, t is a Type variable containing a class, such as WalkCommand. You're getting the PropertyInfo object for the Prefix property on that class, then trying to use GetValue() to read the value of that property from an instance of a WalkCommand object.

The problem is that you aren't passing GetValue() an instance of the WalkCommand class, you're passing it a Type so Reflection is then throwing this exception.

There are a few of ways to deal with this:

1) Create an instance of each type on the fly, just to read its prefix (I really wouldn't recommend doing this):

var instance = currentAssembly.GetTypes()
    .Where(t => t.IsSubclassOf(typeof(CommandBase)) && !t.IsAbstract)
    .Select(t => new { t, i = (CommandBase)Activator.CreateInstance(t) })
    .Where(x => x.t.GetProperty("Prefix").GetValue(x.i).ToString() == prefix)
    .Select(x => x.i)
    .SingleOrDefault();

return instance;

2) Change the whole thing over to use Attributes, such as in SwiftingDuster's answer

3) Use a static constructor to create a Dictionary that maps the string prefices to the concrete types. Reflection is expensive, and the classes aren't going to change (unless you're dynamically loading assemblies), so do it once.

We could do this by (ab)using my earlier "create an instance to throw it away" code, so we're still creating an instance of each class just to read the property, but at least we only do it once:

static Dictionary<string, Type> prefixMapping;

static Program()
{
    prefixMapping = currentAssembly.GetTypes()
        .Where(t => t.IsSubclassOf(typeof(CommandBase)) && !t.IsAbstract)
        .Select(t => new { t, c = (CommandBase)Activator.CreateInstance(t) })
        .ToDictionary(x => x.t.GetProperty("Prefix").GetValue(x.c).ToString(), x => x.t);
}

static CommandBase GetInstance(string prefix)
{
    Type concreteType;
    if ( prefixMapping.TryGetValue(prefix, out concreteType) )
    {
        return (CommandBase)Activator.CreateInstance(concreteType);
    }
    return default(CommandBase);
}

Note that this will throw a really horrible exception if you have multiple classes with the same prefix, and as it would be an exception in a static constructor, it'll likely explode your application. Either catch the exception (which will leave prefixMapping as null), or modify the Linq expression to only return one type for each prefix (as below).

4) Use both the Attribute method from SwiftingDuster and also the precomputation of the dictionary. This would be my preferred solution:

static Dictionary<string, Type> prefixMapping;

static Program()
{
    prefixMapping = currentAssembly.GetTypes()
        .Where(t => t.IsSubclassOf(typeof(CommandBase)) && t.IsDefined(typeof(CommandAttribute), false) && !t.IsAbstract)
        .Select(t => new { t, p = t.GetCustomAttribute<CommandAttribute>().Prefix })
        .GroupBy(x => x.p)
        .ToDictionary(g => g.Key, g => g.First().t);
}

static CommandBase GetInstance(string prefix)
{
    Type concreteType;
    if ( prefixMapping.TryGetValue(prefix, out concreteType) )
    {
        return (CommandBase)Activator.CreateInstance(concreteType);
    }
    return default(CommandBase);
}

Hope this helps