Reputation: 1413
I am working on a piece of C# web application that has a page (MainPage) with a small area to display a gadget. There are several kinds of gadgets implementing a main interface (IMainInterface) and an optional interface (IOptionalInterface).
In MainPage, when interacting with the gadget class, it uses following syntax:
MyAPI api = new MyAPI();
api.SomeMethod(gadgetClassName, param);
And in api.SomeMethod(...), it does following:
// use reflection to get an IMainInterface based on gadgetClassName
Type t = Type.GetType(gadgetClassName);
IMainInterface gadget = (IMainInterface)t.InvokeMember(
gadgetClassName,
BindingFlags.CreateInstance,
null,
null,
null);
return gadget.SomeMethod(param)
Looking at this MyAPI class, it contains a whole bunch of methods that map to the corresponding methods defined in IMainInterface and IOptionalInterface.
My question, is this MyAPI class really neccesary? Wouldn't it be less overhead if MainPage accesses interfaces(IMainInterface and IOptionalInterface) directly?
Update: seeing some of the answers, I realized that I wasn't explicit that the "several kinds of gadget" means different classes (e.g. CalendarGadget, TaskGadget).
Update 2: Added more code sample
Upvotes: 1
Views: 216
Reputation: 18061
The MyApi class looks like it's shielding you from using reflection to create the object, null checks if the optional interface isn't in use, and probably the whole fact of using interfaces in general. Without all the code it's conjecture. As Dzmitry Huba points out, mixing a factory and a wrapper is a bad smell, and you should try to refactor that out.
static class GadgetFactory
{
public static IMainInterface GetGadget(string className)
{
(IMainInterface)Activator.CreateInstance(Type.GetType(className))
}
}
A factory decouples the logic of creation, but it should only be responsible for creation.
Q: Is there any logic in myAPI, or is it just creating and then dispatching if the gadget supports the interface?
If MyApi doesn't have any logic, it's hard to see why it's necessary. Perhaps the writer of MyApi didn't realize at the time that you can cast to the other interface when needed. I have a hunch that they were trying to shield junior developers from interfaces.
// basic use of interfaces
IMainInterface gadget = GadgetFactory.GetGadget("Gadgets.Calendar");
gadget.SomeMethod();
IOptionalInterface optional = gadget as IOptionalInterface;
if( optional != null ) // if optional is null, the interface is not supported
optional.SomeOptionalMethod();
A relevant SO question Difference between Activator.CreateInstance() and typeof(T).InvokeMember() with BindingFlags.CreateInstance
Upvotes: 2
Reputation: 217351
Yes, from the description in your question, I'd say reflection or any other indirection mechanism is unnecessary. I'm not sure if that answers your question though?
// in MainPage:
IList<IMainInterface> gadgets = new List<IMainInterface>
{
(IMainInterface)Activator.CreateInstance(Type.GetType("Gadgets.CalendarGadget")),
(IMainInterface)Activator.CreateInstance(Type.GetType("Gadgets.TaskGadget")),
};
Upvotes: 1
Reputation: 731
Generally this is not bad. But I guess that this is complete overkill for your intentions. I would suggest to borrow something from IOC.
var api = new MyAPI(new GadgetClass());
api.SomeMethod(parameters);
This would make your life much less complicated.
Hope this helps
Upvotes: 0
Reputation: 4521
It seems the author of MyApi mixed factory and consumer. I do not see any reason to access interface members indirectly when it is defined at compile time.
Upvotes: 1