tags:

views:

133

answers:

4

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

+1  A: 

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")),
};
dtb
thanks for the reply, please see my Update for some clarification.
Bill Yang
I still don't get what you're really asking, but I've updated my answer to have more than one gadget...
dtb
hmmm, interesting solution, but how do I call gadget.SomeMethod() when gadget is determined at runtime?
Bill Yang
I've updated my answer to create an instance of a gadget at runtime from its assembly-qualified class name.
dtb
+1  A: 

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.

Dzmitry Huba
A: 

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

Dejan
thanks for the reply, please see my Update for some clarification.
Bill Yang
I don't think there's enough information in the question to reach this conclusion. How does you answer help if MainPage can't instantiate the class? If it's myAPI that's instantiating the class, then in effect, this is already a variant of IOC.
Robert Paulson
The totally purist way would be to have no NEW in the code. All would be done using a IOC container. So the code posted up there is just a little sample.
Dejan
+2  A: 

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

Robert Paulson
That was very well explained sir! Now to think about it, the author was probably intended to shield junior developers from underlying implementation (this is probably more of a subjective matter) and as a factory. I'm going to put it on my to-be-refactored list. Thanks a lot!
Bill Yang