16

I have some legacy code with a method foo which has 700+ overloads:

[DllImport("3rdparty.dll")]
protected static extern void foo(int len, ref structA obj);
[DllImport("3rdparty.dll")]
protected static extern void foo(int len, ref structB obj);
[DllImport("3rdparty.dll")]
protected static extern void foo(int len, ref structC obj);
//and 700 similar overloads for foo...

I'd like to expose these overloaded methods through a single method using generics:

public void callFoo<T>(int len)
    where T : new()  //ensure an empty constructor so it can be activated
{
   T obj = Activator.CreateInstance<T>(); //foo expects obj to be empty, and fills it with data
   foo(len, ref obj);

   //...do stuff with obj...
}

Unfortunately this returns the errors: "The best overloaded method match for 'foo(int, ref StructA)' has some invalid arguments" and "cannot convert from 'ref T' to 'ref StructA'".

Is there an elegant way to achieve this?

Iain Sproat
  • 5,210
  • 11
  • 48
  • 68
  • Are the types `classA`, `classB` part of a class hierarchy? If so, can you explain the structure? – Oded Oct 11 '10 at 10:41
  • 700 overloads? Quite large for a class. – TalentTuner Oct 11 '10 at 10:41
  • Uhu, 700 overloads? You sure you want to add another layer of complexity into that? – Makach Oct 11 '10 at 10:46
  • 2
    @Makach - sometimes, the first step in making something simpler is to make it a bit more complex (adding a layer of indirection, for instance). – Oded Oct 11 '10 at 10:47
  • Yeah, at least 700 and has potential to grow! It's a C dll created by a 3rd party vendor, so I don't have any control over that I'm afraid. I should point out that each of the foo methods is actually a Pinvoke call. – Iain Sproat Oct 11 '10 at 10:53
  • @Oded - there is no hierarchy. all the classes (classA, classB etc.) are entirely independent from each other. – Iain Sproat Oct 11 '10 at 11:00
  • @Oded yes, but sometimes maybe refactoring is better than hiding what might be a problem. – Makach Oct 11 '10 at 18:08
  • 2
    @Makach - and if he has time constraints that don't allow him to refactor now? Or he is doing his best to put the system under test before refactoring? You can make blank statements about things, but without knowing other constraints, don't assume your solution is the best for his situation. – Oded Oct 11 '10 at 18:46

4 Answers4

8

I was hoping that dynamic would help here, but it doesn't like the ref. Anyway, reflection should work:

public T callFoo<T>(int len)
    where T : new()  //ensure an empty constructor so it can be activated
{
   T obj = new T();
   GetType().GetMethod("foo", BindingFlags.Instance | BindingFlags.NonPublic,
       null,  new[] { typeof(int), typeof(T).MakeByRefType() }, null)
       .Invoke(this, new object[] { len, obj });
   return obj;
}

Here's an optimized version that only does the reflection once; should be much faster:

class Test
{

    protected void foo(int len, ref classA obj){}
    protected void foo(int len, ref classB obj){  }
    protected void foo(int len, ref classC obj){}
    static readonly Dictionary<Type, Delegate> functions;
    delegate void MyDelegate<T>(Test arg0, int len, ref T obj);
    static Test()
    {
        functions = new Dictionary<Type, Delegate>();
        foreach (var method in typeof(Test).GetMethods(BindingFlags.NonPublic | BindingFlags.Instance))
        {
            if (method.Name != "foo") continue;
            var args = method.GetParameters();
            if (args.Length != 2 || args[0].ParameterType != typeof(int)) continue;
            var type = args[1].ParameterType.GetElementType();
            functions[type] = Delegate.CreateDelegate(
                typeof(MyDelegate<>).MakeGenericType(type), method);
        }
    }
    public T callFoo<T>(int len)
        where T : new()  //ensure an empty constructor so it can be activated
    {
        T obj = new T();
        Delegate function;
        if (!functions.TryGetValue(typeof(T), out function)) throw new NotSupportedException(
             "foo is not supported for " + typeof(T).Name);
        ((MyDelegate<T>)function)(this, len, ref obj);
        return obj;
    }
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • This works well for instance methods, but unfortunately not for static methods i.e. protected static void foo(int len, ref classA obj){}, I get a System.TypeInitializationException error. (the BindingFlags have been changed from .Instance to .Static already) – Iain Sproat Oct 11 '10 at 11:54
  • @sprocketonline - to handle static you'll need to take out the arg0; are all static? Or is there a mixture of both? – Marc Gravell Oct 11 '10 at 12:11
  • all are static - they're all P/Invoke calls to the external C library. – Iain Sproat Oct 11 '10 at 12:45
  • 1
    @sprocketonine - I'm away from pc, but: change the delegate type definition to remove the arg0 param, change the bindings to static, and don't pass "this" into the invoke. That should do it. – Marc Gravell Oct 11 '10 at 14:46
5

First - since you have where T : new()
you can just state T obj = new T(); instead of T obj = Activator.CreateInstance<T>();
Now, for the other issue, have a lot of functions like this in one class is chaos.
I would define an interface

public interface IFoo
{
   void foo(int len);
}

and make all the classes implement it. And then:

public void callFoo<T>(int len)
    where T : IFoo, new()  //ensure an empty constructor so it can be activated
{
   T obj = new T();
   obj.foo(len);
}
Itay Karo
  • 17,924
  • 4
  • 40
  • 58
5

You can do this by taking care of the marshaling yourself instead of leaving it to the P/Invoke marshaller. Redeclare foo like this:

    [DllImport("3rdparty.dll")]
    private static extern void foo(int len, IntPtr obj);

Which now allows you to define a generic method:

    protected void foo<T>(ref T obj) {
        int len = Marshal.SizeOf(obj);
        IntPtr mem = Marshal.AllocCoTaskMem(len);
        try {
            Marshal.StructureToPtr(obj, mem, false);
            foo(len, mem);
            // Optional:
            obj = (T)Marshal.PtrToStructure(mem, typeof(T));
        }
        finally {
            Marshal.FreeCoTaskMem(mem);
        }
    }

If perf is critical then you can speed it up by keeping the memory allocated by AllocCoTaskMem around, growing it only if needed. It isn't clear from your question whether the C functions updates the passed structure, you can omit the PtrToStructure call if it doesn't.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Yes, the C function does update the passed structure. This solution looks elegant, but unfortunately results in the following error: "System.AccessViolationException : Attempted to read or write protected memory. This is often an indication that other memory is corrupt." – Iain Sproat Oct 11 '10 at 13:47
  • Hmm, should work. Are these structures or classes that you are passing? If you pass class objects then you need a pointer-to-a-pointer, ref IntPtr. – Hans Passant Oct 11 '10 at 13:56
  • Good point - it would be structs I'm passing. I've updated the question to reflect that. – Iain Sproat Oct 11 '10 at 14:46
  • 1
    Oh rats, I reversed the StructureToPtr/PtrToStructure calls. Code snippet updated. – Hans Passant Oct 11 '10 at 14:52
2

I'm afraid that you cannot use generics in a way you want to here. The reason is that the generic method needs to be compiled to IL and it needs to resolve the overload at compile time. At that point, it doesn't really know which overload to choose, because this is runtime information.

If you have as many overloads as you say, then I would really consider using some better abstraction. For example, implement your foo method as a member of some interface that is implemented by all the classes. If you provide more details, I'm sure people here can give advice on better design.

If you really need to do it this way, then you could probably use something like Dictionary<Type, SomeDelegate<int, obj> and store all foo methods in a dictionary. The callFoo method would simply perform a lookup:

public void callFoo<T>(int len)  where T : new()
{ 
   T obj = Activator.CreateInstance<T>();
   fooDictionary[typeof(T)](len, obj);
   // ...
} 

Then the only problem would be, how to add all of them to the dictionary. You can probably do that simply by hand, in static constructor of each class or dynamically using reflection.

Tomas Petricek
  • 240,744
  • 19
  • 378
  • 553