How can I get rid of the reflection invoke in this piece of code?

Question

I'm building a simple "bus" as a proof of concept. I do not need anything complicated but am wondering how best to optimise the following piece of code. I'm using Autofac as a container to resolve commands as open generics but actually executing the commands is currently being done via reflection as the incoming command cannot be cast to a concrete type in the code. See code - marked up with // BEGIN // END - this is currently being done with reflection. Is there a way to do this without using reflection?

// IoC wrapper
static class IoC {
    public static object Resolve(Type t) {
        // container gubbins - not relevant to rest of code.
    }
}

// Handler interface
interface IHandles<T> {
    void Handle(T command);
}

// Command interface
interface ICommand {
}

// Bus interface
interface IBus {
    void Publish(ICommand cmd);
}

// Handler implementation
class ConcreteHandlerImpl : IHandles<HelloCommand> {
    public void Handle(HelloCommand cmd) {
        Console.WriteLine("Hello Command executed");
    }
}

// Bus implementation
class BusImpl : IBus {
    public void Publish(ICommand cmd) {
        var cmdType = cmd.GetType();
        var handler = IoC.Resolve(typeof(IHandles<>).MakeGenericType(cmdType));
        // BEGIN SLOW
        var method = handler.GetType().GetMethod("Handle", new [] { cmdType });
        method.Invoke(handler, new[] { cmd });
        // END SLOW
    }
}

Adam Ralph · Accepted Answer · 2011-10-15T23:51:56.713

How about this (only the changed parts shown):-

// Handler interface
interface IHandles<T> where T : ICommand {
    void Handle(T command);
}

// Bus interface
interface IBus {
    void Publish<T>(T cmd) where T : ICommand;
}

// Bus implementation
class BusImpl : IBus {
    public void Publish<T>(T cmd) where T : ICommand {
        var handler = (IHandles<T>)IoC.Resolve(typeof(IHandles<T>));
        handler.Handle(cmd);
    }
}

The key here is to make the Publish method generic, which means that you get a type reference T to the type of the command, which can then be used to make the cast. The type parameter constraints simply ensure that only an ICommand can be passed, as before.

BTW - I've tested this and it works, here's the full code:-

public static void Main(){
   new BusImpl().Publish(new HelloCommand());
}

// IoC wrapper
static class IoC {
    public static object Resolve(Type t) {
        return new ConcreteHandlerImpl();
    }
}

// Handler interface
interface IHandles<T> where T : ICommand {
    void Handle(T command);
}

// Command interface
interface ICommand {
}


// Handler implementation
class ConcreteHandlerImpl : IHandles<HelloCommand> {
    public void Handle(HelloCommand cmd) {
        Console.WriteLine("Hello Command executed");
    }
}

public class HelloCommand:ICommand{}

// Bus interface
interface IBus {
    void Publish<T>(T cmd) where T : ICommand;
}

// Bus implementation
class BusImpl : IBus {
    public void Publish<T>(T cmd) where T : ICommand {
        var handler = (IHandles<T>)IoC.Resolve(typeof(IHandles<T>));
        handler.Handle(cmd);
    }
}

-- UPDATE --

As pointed out by Peter Lillevold, you should also think about adding a type parameter to your IOC container method as follows:-

// IoC wrapper
static class IoC {
    public static T Resolve<T>() {
        ...
    }
}

this will simplify your caller like so:-

// Bus implementation
class BusImpl : IBus {
    public void Publish<T>(T cmd) where T : ICommand {
        var handler = IoC.Resolve<IHandles<T>>();
        handler.Handle(cmd);
    }
}

This is a side point to your original question, but would seem a sensible design for the IOC interface.

Adding generic constraints now and testing - thanks for the reply. Will post with success (hopefully). — Deleted, Oct 15 '11 at 14:11
Excellent - thank you so much. Works perfectly! I can see how the constraints allow the type to be coerced without breaking the runtime or compiler as well. Good solution. Thanks again. — Deleted, Oct 15 '11 at 14:16
hm... am I missing something here? why not just do `var handler = IoC.Resolve>();` ? You already know the type `T`... no need for dynamically making the generic type... — Peter Lillevold, Oct 15 '11 at 22:59
@Peter - that's a good point, and a valid simplification (I've updated the code to reflect it). But it only has relevance to the resolution of the type via the IOC container, which is side point. The key to avoiding the reflection in the original code, which was the reason the question was asked, is to add the type parameter to the `Publish` method. Moreoever, if the IOC container method was re-written with a type parameter the code could just become `var handler = IoC.Resolve>();` — Adam Ralph, Oct 15 '11 at 23:38
@Peter - I've added an update to illustrate this - thanks for the motivation ;-) — Adam Ralph, Oct 15 '11 at 23:52

CRice · Answer 2 · 2011-10-15T13:20:36.500

0

Does this work? Your IoC is returning object, consider returning T instead then you wont have to handle type ambiguity as done below.

public void Publish(ICommand cmd) {
    var cmdType = cmd.GetType();
    var handler = IoC.Resolve(typeof(IHandles<>).MakeGenericType(cmdType)) as IHandles<ICommand>;
    if (handler != null)
    {
        // BEGIN SLOW
        handler.Handle(command);
        // END SLOW
    }
    //else throw some exception
}

edited Oct 15 '11 at 13:20

answered Oct 15 '11 at 13:14

CRice

12,279
7
57
84

No - that will never run what is in the null check block. Resolve returns an Object which cannot be cast without throwing an exception. That is where I started. It appears you can't up-cast the impl so it is callable hence the reflection mess. You actually get an invalid cast exception if you force it - in this case Unable to cast type "ConcreteHandlerImpl" to type "IHandles`1[ICommand]". I'm not sure if you can do this with covariance/contravariance somehow. – Deleted Oct 15 '11 at 13:21
@Chris Smith: what does your ConcreteHandlerImpl look like? – Adam Ralph Oct 15 '11 at 13:31
Hard to see what's going on, your IoC is probably getting a type that does not use the interface then. – CRice Oct 15 '11 at 13:33
@CRice: It is definitely returning a type which implements the interface according to the debugger. – Deleted Oct 15 '11 at 13:34
Yes it's based on the returned type if ConcreteHandlerImpl implemented Handles there would not have been a problem but a good solution has been posted now anyway. – CRice Oct 16 '11 at 04:33

How can I get rid of the reflection invoke in this piece of code?

2 Answers2