snippetcsharpMinor
IEnumerable of classes that implement a given interface at runtime
Viewed 0 times
implementruntimeinterfacethatclassesgivenienumerable
Problem
I am implementing the command pattern in a project that I am working on and I have an interface,
When I run the application I want to dynamically gather all classes in the current assembly that implement that interface, thus producing a list of all possible commands.
So I came up with the method below. This was the first thing that I could get to work. Is there a better way to do this?
ICommandFactory that all of my commands are implementing. When I run the application I want to dynamically gather all classes in the current assembly that implement that interface, thus producing a list of all possible commands.
So I came up with the method below. This was the first thing that I could get to work. Is there a better way to do this?
private static IEnumerable GetAvailableCommands()
{
var interfaceType = typeof (ICommandFactory);
var implementors = Assembly.GetExecutingAssembly().GetTypes().Where(
i => interfaceType.IsAssignableFrom(i) && i != interfaceType);
var commands = new List();
foreach(var implementor in implementors)
{
var ctor = implementor.GetConstructor(Type.EmptyTypes);
if (ctor == null) continue;
var instance = ctor.Invoke(null) as ICommandFactory;
if (instance == null) continue;
commands.Add(instance);
}
return commands;
}Solution
I only have a few minor comments about your code:
-
This code:
has the potential to mask a bug. Imagine you add a constructor to one of your command-factory types and accidentally forget that this implicitly removes the default constructor. This code will then silently swallow this mistake and simply not list that particular command factory. It would be preferable to throw in this case so that you notice it immediately. Even better, of course, would be to use a post-build event to turn this error into a compile-time check so that you can’t even run the program when it is invalid.
-
This code:
has exactly the same problem, but the consequences are much more subtle. The
-
Your code does not allow for a class that implements
-
Very very minor nitpick:
You should perform the simpler check first:
-
This code:
var ctor = implementor.GetConstructor(Type.EmptyTypes);
if (ctor == null) continue;has the potential to mask a bug. Imagine you add a constructor to one of your command-factory types and accidentally forget that this implicitly removes the default constructor. This code will then silently swallow this mistake and simply not list that particular command factory. It would be preferable to throw in this case so that you notice it immediately. Even better, of course, would be to use a post-build event to turn this error into a compile-time check so that you can’t even run the program when it is invalid.
-
This code:
var instance = ctor.Invoke(null) as ICommandFactory;
if (instance == null) continue;has exactly the same problem, but the consequences are much more subtle. The
if (instance == null) condition should never fire because the code was written so that the instance cannot be of any other type. Imagine you have a bug in that code. This code swallows this bug and you won’t notice it directly. You will only notice some subtle/weird effects that occur much later, and it will be a pain to trace it back to this code here. You could make it throw like above, but personally I think you should change this to var instance = (ICommandFactory) ctor.Invoke(null); so that it will automatically throw if it’s not of the right type.-
Your code does not allow for a class that implements
ICommandFactory but is not intended to appear in your UI. If you are sure that you don’t want that to be possible, then your code is fine. Otherwise, I would declare a custom attribute — either one that allows you to explicitly exclude an ICommandFactory from appearing in the UI, or one that is required on all the ICommandFactory types to be included.-
Very very minor nitpick:
i => interfaceType.IsAssignableFrom(i) && i != interfaceTypeYou should perform the simpler check first:
i => i != interfaceType && interfaceType.IsAssignableFrom(i)Code Snippets
var ctor = implementor.GetConstructor(Type.EmptyTypes);
if (ctor == null) continue;var instance = ctor.Invoke(null) as ICommandFactory;
if (instance == null) continue;i => interfaceType.IsAssignableFrom(i) && i != interfaceTypei => i != interfaceType && interfaceType.IsAssignableFrom(i)Context
StackExchange Code Review Q#928, answer score: 5
Revisions (0)
No revisions yet.