views:

40

answers:

4

I have some old C# plugin code that was implemented strictly with Reflection. In fixing some C# 2.0 -> 4.0 compatibility issues ("Load from Remote Source") I've decided to get rid of the old reflection code and replace it with an interface. The interface is needed because the Plugins now need to be loaded into their own AppDomain and then marshalled across.

I can go through the source for hundreds of plugins and simply make them all implement the new IPlugin interface with a simple search-and-replace. This works nicely except in one crucial place. And I'm looking for an easy out.

The method RunPlugin() can be implemented in one of two ways, but never both: Either with an argument or without. If I include this in the interface, I'd have to implement both in each of the plugins. The caller calls the one or no argument method based on which one is implemented. The calling assembly figures this out now by reflection.

To avoid that I can create a wrapper class for the plugins, that wrapper implements the interface, but then I'd have to heavily edit each of the plugins to include an override for each of the API's many methods.

Some sample code (this doesn't necessarily work! It's all in transition right now!):

The interface (sample):

// In IPlugin.cs / IPlugin.dll
namespace Plugin
{
    public interface IPlugin
    {
           // Many, many happy API things like this...
           void SetupOptions(Hashtable options);
           // (examples elided)

           // And then these two... either one or the other
           // is implemented, but not both.
           XmlDocument RunPlugin(Updater u);
           XmlDocument RunPlugin();
    }
}

The called Assembly... I have lots of these. I can add the ": IPlugin" fairly easily. This won't compile, obviously, because it doesn't implement the one-argument RunPlugin().

namespace Plugin
{
      public class FileMaintenance : IPlugin
      {
          public void SetupOptions(Hashtable options)
          {  // Code elided
          } 

          public XmlDocument RunPlugin()
          {  // Code elided
          }
      }
}

And finally, the calling code. This is actually how it used to look, back in the reflection code:

public XmlDocument RunPlugin(PluginRunner.Updater u)
{
    Type [] paramTypes = new Type [0];
    MethodInfo runInfo = repType.GetMethod("RunPlugin", paramTypes);
    if (runInfo == null)
    {
        paramTypes = new Type [1];
        paramTypes[0] = u.GetType();
        runInfo = repType.GetMethod("RunPlugin", paramTypes);
        if (runInfo == null)
            throw new Exception;
    }
    Object[] parameters;
    if ( paramTypes.Length == 0)
        parameters = new Object[0];
    else
    {
        parameters = new Object[1];
        parameters[0] = u;
    }
    Object returnVal;
    try 
    {
        returnVal = runInfo.Invoke(repObj,parameters);
    }
    catch (Exception e)
    {
            }
         // Rest of code omitted
 }

Remember: I'm looking for a nice balance between the right way to fix this old code, and doing the minimal amount of editing code by hand.

+3  A: 

I would advocate creating two additional interfaces:

public interface IRunnablePlugin : IPlugin
{
    XmlDocument RunPlugin();
}

public interface IParamRunnablePlugin : IPlugin
{
    XmlDocument RunPlugin(object parameter);
}

Then have all of your plugins implement one or the other. The only time you'll have to make a distinction is when actualling calling RunPlugin. All other times you can refer to it as a simple IPlugin.

For example, to perform the call, you'd do something like this:

IPlugin plugin = ...;

IRunnablePlugin runnable = plugin as IRunnablePlugin;
IRunnableParamPlugin param = plugin as IRunnableParamPlugin;

XmlDocument output;

if(param != null)
{
    output = param.RunPlugin(parameter);
}
else if(runnable != null)
{
    output = runnable.RunPlugin();
}
else
{
    throw new InvalidOperationException();
}

Note that there is technically no limitation that permits the developer to implement only one of the versions, but that should hopefully not be an issue. In this code, you're checking for the presence of the parameterized version first, then the parameterless version, then throwing an exception if neither is found.

Adam Robinson
And so when calling RunPlugin, how do I figure out which of the interfaces are implemented? Because I won't actually know at the caller unless I somehow glean that from the plugin.
clintp
@clintp: See the edit for more information.
Adam Robinson
+1  A: 

Unfortunately having one or the other RunPlugin methods was the Achilles heal that got added to this design from the getgo. That was a precarious design decision, it's going to be hard to deal with it.

One possibility is to add both overloads to IPlugin, and let plugins indicate the one they didn't implement by throwing NotImplementedException. That probably doesn't buy you much, it might not buy you anything.

Another possibility is two interfaces, IPlugin and IPluginWithArgs, and detect which interface a given plugin is implementing and go from there. Also ugly.

Another possibility is an extension method public static void RunPlugin(this IPlugin plugin) that basically hides and tidies up the reflection you've already got going. I don't think this buys you anything either.

Matt Greer
A: 

You could have one base interface like so:

public interface IPlugin {
           // Many, many happy API things like this...
           void SetupOptions(Hashtable options);
           // (examples elided)

           XmlDocument RunPlugin();
}

//
// And another interface extending the IPlugin that defines the update version
//

public interface IUpdaterPlugin : IPlugin {
           XmlDocument RunPlugin(Updater u);

}

To run the appropriate plugin...

if( plugin is IUpdaterPlugin)
    plugin.RunPlugin(updater);
else if(plugin is IPlugin)
    plugin.RunPlugin();
Adrian Regan
A: 

So, you say...

To avoid that I can create a wrapper class for the plugins, that wrapper implements the interface, but then I'd have to heavily edit each of the plugins to include an override for each of the API's many methods.

I'm not quite sure I understand the problem with the wrapper class, but we might be thinking about different things.

The wrapper class I'm thinking about would take the interface with zero parameters and make it look like the interface with one parameter. That parameter would simply be ignored when "RunPlugin" was called.

This would remove any branching on types for your interfaces and you'd only have to create this one class - it could wrap any instance of the no-parameter interface. So there shouldn't be any special code needed for each plugin. Just create the wrapper when you create the instance of the plugin.

(By the way, this is the Adapter pattern.)

Joseph
The wrapper class would still have to do some kind of shenanigans to determine whether the ultimate, actual implementation had the Zero or One parameter version of the method. Thus I'd still need the reflection, different interfaces, or editing each of the implementation classes to add this additional call. With a wrapper I'm just moving the complexity down from the original caller into the wrapper class.
clintp
@clintp: I might be misunderstanding the problem, but what I'm suggesting is that the wrapper class only wraps implementations with zero parameters. You're right in that something, at some time, has to know whether you are dealing with a single-parameter or no-parameter interface. I would expect that you SHOULD be able to do that pretty trivially when the plugin is actually instantiated...but maybe I just don't understand your situation well enough.
Joseph
@clintp: In case this isn't clear...the wrapper implements the normal one-parameter interface. Once the zero-parameter interfaces are exposed as one-parameter interfaces through the wrapper, everything looks like a one-parameter interface.
Joseph