Bill Yang
Bill Yang

Reputation: 1413

Purpose of having API wrapped around interface

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

Answers (4)

Robert Paulson
Robert Paulson

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

dtb
dtb

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

Dejan
Dejan

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

Dzmitry Huba
Dzmitry Huba

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

Related Questions