Use runtime code gen using methodInfo.CreateGenericMethod()

Roy,

Are you talking about reflection emit?

I can't understand how this would use reflection only once.

void Dispatch<T>(T msg) where T : AbstractMessage, new

{

}

Something on these lines?

The syntax might not be right but I assume you understand what I mean.

If to consider task as real - you could use interface instead of AbstractMessage and avoid this challange at all.

If to consider this as a task to be solved, see above.

Main idea is to cache created delegates once and after that use direct method calls without reflection cost.

Full implementation source code:

using System;

using System.Collections.Generic;

using System.Reflection;

namespace ReflectionWithNoCost

{

class Program

{

    /// <summary>

    /// Delegates cache

    /// </summary>

    private static Dictionary<string, Action<IMessage>> _cache = new Dictionary<string, Action<IMessage>>();


    static void Main(string[] args)

    {

        Dispatch(new ConcreteMessage());

        Dispatch(new AbstractMessage());

        Console.ReadLine();

    }


    public static void Dispatch(AbstractMessage msg)

    {

        var concreteType = msg.GetType();

        if (!_cache.ContainsKey(concreteType.FullName))

        {

            _cache.Add(concreteType.FullName,

                (Action<IMessage>)Delegate.CreateDelegate(typeof(Action<IMessage>), typeof(Program).GetMethod("DispatchGeneric").MakeGenericMethod(concreteType)));

        }

        _cache[concreteType.FullName](msg);

    }


    public static void DispatchGeneric<T>(IMessage msg)

        where T : IMessage

    {

        IMessageHandler<T> handler = new MyHandler<T>();

        handler.Handle((T)msg);

    }

}


public interface IMessageHandler<T>

    where T : IMessage

{

    void Handle(T msg);

}


public interface IMessage

{

}


public class AbstractMessage : IMessage

{

}


public class ConcreteMessage : AbstractMessage, IMessage

{

}


public class MyHandler<T> : IMessageHandler<T>

        where T : IMessage

{

    #region IMessageHandler<T> Members


    public void Handle(T msg)

    {

        Console.WriteLine(String.Format("Message of type {0} dispatched", typeof(T).FullName));

    }


    #endregion

}

}

Kalpesh,

it still doesn't meet the requirement of having reflection only one time

I think this will work, not sure though. If you were so concerened about calling GetType you may just want to call it once and use a single var... :)

IMessageHandler<AbstractMessage> handler = new MyHandler<AbstractMessage>();

handler.Handle(msg);

Use LCG to create dynamic factory method that calls the ctor and make a delegate out of it. This happens on the first time and the delegate is cached.

Every time after that, I would just call the cached delegate.

how about

handler = new MyHandler<AbstractMessage>();

normally this won't make any difference, it only does if Handle() uses the type of the message for some significant action, and does so using typeof(T) instead of msg.GetType(). why is IMessageHandler generic?

You should implement a non-generic interface on messagehandler. That requires 0 times reflection and works every time. Interfaces are a good way of generic programming without generics.

Another way is to cache the MakeGenericType output.

I meant you can create generic method implementations from a base generic method like this:

        MethodInfo info = MethodBase.GetCurrentMethod() as MethodInfo;

MethodInfo customGenericMethodPerType = info.MakeGenericMethod(typeof(whatever))

assumign Methodbase.Current is a GenericMethod<T>

you can also do:

info.GetGenericMethodDefinition().MakeGenericMethod(typeof(whatever) if you are creating it from a specific generic method

I'm sure of all the usage requirements of Dispatch. But here is my attempt. I like that it doesn't use any dictionary, but instead uses the type system itself. I assume this is faster and uses less memory.

A potential issue with my code is that, in the handler, typeof(T) is the compile-time type. But then if you are passing a variable of a given compile time type you usually want it treated as such.

using System;

namespace ConsoleApplication1

{

class Program

{

    static void Main(string[] args)

    {

        var dispatcher = new MessageDispatcher();

        dispatcher.Dispatch(new MessageA());

        dispatcher.Dispatch(new MessageB());

        dispatcher.Dispatch(new MessageC());


        AbstractMessage msg = new MessageA();

        dispatcher.Dispatch(msg);

    }


}


class AbstractMessage { }

class MessageA : AbstractMessage { }

class MessageB : AbstractMessage { }

class MessageC : AbstractMessage { }


class MessageDispatcher

{

    static class HandlerFactory<T>

        where T : AbstractMessage

    {

        public static IMessageHandler<T> CreateHandler()

        {

            return new MyMessageHandler<T>();

        }

    }


    public void Dispatch<T>(T msg)

        where T : AbstractMessage

    {

        HandlerFactory<T>.CreateHandler().Handle(msg);

    }

}


interface IMessageHandler<T>

    where T : AbstractMessage

{

    void Handle(T msg);

}


class MyMessageHandler<T> : IMessageHandler<T>

    where T : AbstractMessage

{

    public void Handle(T msg)

    {

        Console.WriteLine("Handling message of type " + typeof(T).FullName);

    }

}

}

EDIT: I'm NOT sure of all the usage requirements of Dispatch.

I wish I could program in IL so that I would create dynaimc methods one for constructor and one for handle method of the handler. The rest is caching, it seems.

Reshef,

You are right, but the solution can be made simpler...

Stefan,

No, the type of the message is important. My Handler is a sample, real things would be things like MyMsgHandler or MyMsg2Handler

Andrew Davey,

the issue with your code that you mentioned is the main issue of your approach. You Dispatch method can't recognise concrete message types if they are unknown during compile time. Initial challenge question and example doesn't make sense if to consider that we know concrete message type. Why would author call msg.GetType() if he knew concrete message type?

Frans,

Non generic would defeat the purpose I am trying to achieve here.

I want generic here, because that has meaning.

pb,

I am using the generic type

Roy,

But the current method is not generic, I want to call a method on a generic type.

Andrew,

Dispatch cannot be a generic method call

If we consider the GetType() call for each message, the following solution may be a good starting point:

public interface IMessageHandler<T> where T : AbstractMessage

{

void Handle(T msg);

}

public class AbstractMessage

{

}

public class MessageA : AbstractMessage

{

}

public class MessageB : AbstractMessage

{

}

public class MyHandler<T> : IMessageHandler<T> where T : AbstractMessage

{

public void Handle(T msg)

{

  Console.WriteLine(string.Format("'{0}' handled '{1}' message.", this.GetType(), msg.GetType()));

}

}

public class Dispatcher

{

private delegate void DispatchMessageDelegate(AbstractMessage msg);

private readonly Dictionary<Type, DispatchMessageDelegate> dispatchers = new Dictionary<Type, DispatchMessageDelegate>();

public void Dispatch(AbstractMessage msg)

{

  var dispatcherDelegate = GetDispatchDelegate(msg.GetType());

    dispatcherDelegate(msg);

}

private DispatchMessageDelegate GetDispatchDelegate(Type messageType)

{

  if (!this.dispatchers.ContainsKey(messageType))

  {

      var dispatchDelegate = GenerateDispatchDelegate(messageType);

     this.dispatchers[messageType] = dispatchDelegate;

  }

  return this.dispatchers[messageType];

}

private static DispatchMessageDelegate GenerateDispatchDelegate(Type messageType)

{

  Console.WriteLine(string.Format("Use reflection for '{0}'.", messageType));

  var method = new DynamicMethod(string.Format("Dipatch{0}Message", messageType.Name),

                                   null,

                                   new[]{typeof(AbstractMessage)},

                                   typeof(Dispatcher));


  var gen = method.GetILGenerator();


  var closedHandlerType = typeof (IMessageHandler<>).MakeGenericType(messageType);

  var constructorInfo = typeof (MyHandler<>).MakeGenericType(messageType).GetConstructor(new Type[]{});

  var handleMethod = closedHandlerType.GetMethod("Handle");


  gen.DeclareLocal(closedHandlerType);


  gen.Emit(OpCodes.Nop);

  gen.Emit(OpCodes.Newobj, constructorInfo);

  gen.Emit(OpCodes.Stloc_0);

  gen.Emit(OpCodes.Ldloc_0);

  gen.Emit(OpCodes.Ldarg_0);

  gen.Emit(OpCodes.Castclass, messageType);

  gen.Emit(OpCodes.Callvirt, handleMethod);

  gen.Emit(OpCodes.Nop);

  gen.Emit(OpCodes.Ret);


  return (DispatchMessageDelegate) method.CreateDelegate(typeof (DispatchMessageDelegate));

}

}

[TestFixture]

public class TestHandler

{

[Test]

public void ShouldWork()

{

  var dispatcher = new Dispatcher();

  var messageA = new MessageA();

  var messageB = new MessageB();


  dispatcher.Dispatch(messageA);

  dispatcher.Dispatch(messageA);

  dispatcher.Dispatch(messageA);

  dispatcher.Dispatch(messageB);

  dispatcher.Dispatch(messageB);

  dispatcher.Dispatch(messageB);

}

}

Romain

That's not what I meant :) IMessageHandler<T> implements IMessageHandler, and IMessageHandler has a Handle() method.

Ok, instaed of LCG we can have a static factory method like this:

internal static IMessageHandler<TMessage> FactoryMethod<TMessage>() {

return new MyHandler<TMessage>();

}

By reflection (on the first time) we will use 'MakeGenericMethod' with the type of the message, make a delegate out of it and cache it for each message type. In further calls we will use the cached factory method.

It is like what u did here: http://ayende.com/Blog/archive/2008/01/21/Challenge-Strongly-typing-weakly-typed-code.aspx

Why do you think that Kalpesh's solution is wrong ? As far as I know CLR will create a new type only once no matter how many different message types your application defines because AbstratMessage is a reference type.

BTW This won't compile :

IMessageHandler<msg.GetType()> handler = new MyHandler<msg.GetType()>();

Reshef,

if you want to build a cache, you need the same type for every entry. If you only cache Action<AbstractMessage>, you'll have to create code that casts that AbstractMessage to whatever T really is.

Ayende,

you mean like class MyMsg2Handler : IMessageHandler<MyMsg2> ?

How would that affect the Dispatch method? Should it handle various message handler types, like, using a dictionary that maps message types to handler types? Or do you mean that MyHandler could be replaced by one single other class? What difference would that make?

If it's necessary, I'd just create a c# 3.0 expression, compile it and store the resulting delegate in a dictionary. Composing and compiling Expressions is easier, more readable and more robust than Reflection.Emit.

• define a generic dispatch method (e.g. DoDispatch<T>) that does what your Dispatch method does, only statically typed, so you can limit code generation to calling that method

• create a lambda expression for (AbstractMessage) m => DoDispatch ((T) m) (writing this in C# syntax is a bit misleading, since that would require a return type, but dynamically it's legal)

• compile it to an Action<AbstractMessage>

• store it in a cache<Type, Action<AbstractMessage>

Just a few lines of code. I still don't get the need for doing this, though.

@Pawel Pabich

If this would compile, there wont be this challenge practice. What we need to do is to find a way to create MyHandler'1 instance with msg.GetType() parameter with using generics in only one call of Dispatch method.

Edit: ..using reflection in only one call of Dispatch method.

I created the non-generic IMessageHandler so we have something to call handlers when the type isn't known at compile-time.

I then created a static class, Messaging, to control the instantiation. Internally it defines a lambda which takes a type and returns a lambda which instantiates the message handler:

Func<Type, Func<IMessageHandler>> _handlerFactory;

I then memoize the lambda so it remembers the lambdas it returns for creating each message type, meeting the 1-time reflection goal.

(see http://www.infoq.com/news/2007/01/CSharp-memory)

You would use this as such:

public void Dispatch(AbstractMessage msg)

{

IMessageHandler handler = Messaging.GetHandler(msg.GetType());

handler.Handle(msg);

}

Source:

public interface IMessageHandler

{

void Handle(AbstractMessage msg);

}

public interface IMessageHandler<T> : IMessageHandler where T : AbstractMessage

{

new void Handle(T msg);

}

public class MessageHandler<T> : IMessageHandler<T>

{

public void Handle(T msg)

{

    ...

}


void IMessageHandler.Handle(AbstractMessage msg)

{

    // Single point of typing failure

    return Handle((T) msg);

}

}

public static class Messaging

{

private static Func<Type, Func<IMessageHandler>> _handlerFactory;


static Messaging()

{

    _handlerFactory = (type => GetHandlerCreator(type)).Memoize();

}


public static IMessageHandler GetHandler(Type messageType)

{

    var creator = _handlerFactory(messageType);


    return creator();

}


private static Func<IMessageHandler> GetHandlerCreator(Type messageType)

{

    var creator = Expression.Lambda<Func<IMessageHandler>>(

            Expression.New(typeof(MessageHandler<>).MakeGenericType(messageType)));


    return creator.Compile();

}

}

Actually, GetHandler's signature should be:

public static IMessageHandler GetHandler(AbstractMessage msg)

...

and the calling code updated accordingly.

Instead of having the generic declaration on the interface, I'd rather have it on the method:

public interface IMessageHandler

{

void Handle<T>(T msg) where T : AbstractMessage;

}

That means that the concrete implementation could look like this:

internal class MyHandler : IMessageHandler

{

public void Handle<T>(T msg) where T : AbstractMessage

{


}

}

And the calling code:

public class Caller

{

public void Dispatch(AbstractMessage msg)

{

  IMessageHandler handler = new MyHandler();

  handler.Handle(msg);

}

}

Doing this means that you don't need to specify the type of msg when you call the Handle method, allowing you to call handle with any sub-class of AbstractMessage. This of course reduce the scope of the generic type to the method level....

Frans,

A single class may implement more than a single IMessageHandler<T>

Romain Verdier,

You have the right approach, but LCG is not required.

Bryan Watts,

Very good. I used a delegate and a manual dictionary, but that is my approach.

I like yours beter

Ayende,

A single class may implement more than a single IMessageHandler<T>

OK, now that's a design where just doing

handler = new MyHandler<AbstractMessage>();

as I initially suggested would not work. I thought it's just

MyHandler<T> : IMessageHandler<T>

But, given

MyHandler : IMessageHandler<Msg1>, IMessageHandler<Msg2>

I don't see how Bryan's solution would work:

public class MessageHandler<T> : IMessageHandler<T> {

void IMessageHandler.Handle(AbstractMessage msg) {

  return Handle((T) msg);

}}

When you're implementing multiple IMessageHandler<T>, there's no T to cast the msg to, which puts us on square one. When you're not, I still maintain you can just call it by passing AbstractMessage (or whatever that class's T is constrained to), unless the implementation does something unlikely.

That's why I suggested coding the entire Dispatch method as a generic Dispatch<T> method and use Expression/Compile only to invoke that method with the correct T.

Mind to explain what I'm missing?

I'd still like to see your solution that involves no LCG at all, btw.

I just noticed that your blog configuration even swallows non-breaking spaces in comments. That's a pain for posting source code.

Stefan,

U can take a look at the method 'GetSecurityKeyProperty' in

https://rhino-tools.svn.sourceforge.net/svnroot/rhino-tools/trunk/rhino-security/Rhino.Security/Security.cs

to see how u can avoid LCG.

Refesh,

That code creates a delegate for this method:

string GetSecurityKeyPropertyInternal<TEntity>()

This is only possible because the generic type does not affect the signature. That's not the case for Handle(T msg), you cannot create an Action<AbstractMessage> for a method that takes a concrete message type, it works only the other way around (contravariance for input parameters)

Stefan Wenig,

You are right, my solution as coded does not facilitate multiple implementations of IMessageHandler<T>.

I was constructing a type hierarchy which allows both generic and non-generic references to the same object. Classes themselves are fully-typed; the only thing which changes is your perspective (the type of your reference).

That said, I may have misunderstood the intent of the class. I was pumping a concrete type through something which has a generic parameter, giving compile-time access to an otherwise unknown type.

Multiple interfaces generally means explicit implementation and multiple pathways. Rather than handling any case, you handle very specific cases:

public class Handles1And2 : IMessageHandler<Msg1>, IMessageHandler<Msg2>

{

void IMessageHandler<Msg1>.Handle(Msg1 msg)

{

HandleMsg1(msg);

}

void IMessageHandler<Msg2>.Handle(Msg2 msg)

{

HandleMsg2(msg);

}

void IMessageHandler.Handle(AbstractMessage msg)

{

if(msg is Msg1)

{

HandleMsg1((Msg1) msg);

}

else if(msg is Msg2)

{

HandleMsg2((Msg2) msg);

}

else

{

// Argument error

}

}

public void HandleMsg1(Msg1 msg)

{

...

}

public void HandleMsg2(Msg2 msg)

{

...

}

}

Bryan,

yes, actually I'm not arguing with your solution, but with Ayendes statements about it. I still maintain, that without multiple implementations of IMessageHandler<T>, you usually don't need any of this. With it (and his intitial question does not show this), you can still go down the generic road like I described, so why code all that non-generic interface stuff and dispatch the call manually?

public static void Dispatch (AbstractMessage msg)

{

  Type T = msg.GetType ();

  Action<AbstractMessage> action;


  if (! _cache.TryGetValue (T, out action) )

  {

    MethodInfo doDispatch = typeof (Program).GetMethod ("DoDispatch", BindingFlags.NonPublic | BindingFlags.Static); // cache this 

    doDispatch = doDispatch.MakeGenericMethod (T);


    // (AbstractMsg m) => doDispatch<T> ((T) m)

    var msgParam = Expression.Parameter (typeof (AbstractMessage), "msg");

    action = Expression.Lambda<Action<AbstractMessage>> (

            Expression.Call (doDispatch, Expression.Convert(msgParam, T)),

            msgParam)

        .Compile();

    _cache.Add (T, action);

  }


  action (msg);

}


private static void DoDispatch<T> (T msg) where T : AbstractMessage

{

  IMessageHandler<T> handler = new MyHandler<T>();

  handler.Handle(msg);

}

that's it, so why bother with anything less?

Stefan,

For the problem stated, we have six of one, a half dozen of the other. I would suggest you rename "DoDispatch" to "Dispatch" and make it public, allowing compile-time consumers to bypass the reflection.

The subtle difference is that your solution is scoped to the method, mine to the class. General instantiation of MessageHandler<T> offers more reuse than the single instance in "Dispatch".

I also prefer to implement any generic class with an untyped interface (IMessageHandler<T> : IMessageHandler). Without, you eventually get into situations where you simply lack the correct nouns!

Bryan,

I'm not going to argue about naming, just like I cannot argue about methods being static, caches being thread-safe or what have you. There's just no information to base those decisions on. I named the thing DoDispatch to have separate names and make the sample more readable, that's all. It's just a quiz, but it seems to be more about reading Ayende's thoughts than about solving the actual problem. Maybe that's what he wanted:

public void Dispatch(AbstractMessage msg, Type handlerType)

{

IMessageHandler<msg.GetType()> handler =(IMessageHandler<msg.GetType()>) Activator.Create (handlerType);

handler.Handle(msg);

}

Because instantiating a handler that implements IMessageHandler<IMsg1> and <IMsg2> via new MyHandler<T> makes little sense to me. That's why I initially recommended to just call Handle<AbstractMessage> and be done with it.

Anyway, the trick is using Expression.Compile for LCG, the rest is beyond this challenge. You could even build it into a reflection utility class that knows nothing about IMessageHandler, and pass DoDispatch as a parameter.

Providing a non-generic interface should be a case by case decision. Once, because YAGNI, and then because we just showed that it's not that hard to work around it if it's missing. Implementing manual dispatching like you did for the non-generic Handle method is tedious, and error-prone if you never really use it. Even if you're into TDD you can easily miss a case. It's just not DRY. (now I'm running out of impressive acronyms...)

Stefan,

I suggested the name change to bring the method in line with the existing public API. I was not commenting on stylistic choice - "Do*" is a well known naming convention for "actual-work" methods.

I agree that this challenge doesn't make sense in the context of multiple implementations - we at least need some mapping of message type to handler type, otherwise how do we know what to instantiate?

As for non-generic interfaces: I consider them a necessary aspect of implementing generic classes. Someone somewhere will curse you because they can't act upon your object in an abstract manner which is meaningful.

I like to think of Eric Lippert's "intent vs mechanism" dichotomy: an untyped interface declares an intent of untyped usage; the same intent cannot otherwise be expressed sans imperative code.

Bryan,

the guy who does the cursing might be myself - I'm not always designing libraries for other people. In that case, I (or a coworker) can add the untyped stuff when I need it and not clutter everything with duplicate stuff just in case. Thus the reference to YAGNI. In other cases, I might choose to use only untyped interfaces, because generics somtimes only add complexity. In a situation like this here, I might end up having only generic interfaces for the beauty and clarity of it, but avoid handling different messages in one type, but chances are I'd rather avoid the generics completely, since the handle method is likely to be invoked in generic handler loops only. I'll stick with case-by-case decisions.

BTW, instead of cursing, one could just write their generic wrappers (like my DoDispatch method) and call them using a more general method, like that one:

  var action = ReflectionUtility.MakeGenericWrapper ((AbstractMessage msg) => Dispatch (msg));

  action (new AbstractMessage ());

  action (new ConcreteMessage ());

Granted, this might be more magic that you'd want to expose the users of your types to, but it can be done :-)

In this very case I'd go with generics, because when I'm asked, out of context, how to make generics fly, getting rid of those generics does not seem like much of an answer. In real life, it may well be.

Another point: When you define an f(X) for every f(T) where T: X, you'll lose type safety. Same name, same signature except for the super type: that will always compile, even if it becomes invalid due to some other changes. The generic method becomes a mere hint via intellisense.

You are correct. The option is in the hands of the user when using an interface reference. The untyped method simply calls the typed method with a cast, so it still buys a single point of typing transition. But it does degrade the API a little.

I use this pattern mostly for writing concrete object hierarchies deriving from a base implementation of the generic interface, something like:

public abstract class ValidationRule<T> : IValidationRule<T>

{

...

}

public class EmailRule : ValidationRule<string>

{

...

}

Now, I can easily reason about the objects comprising the framework, from any perspective.

This pattern makes less sense in the message handler example, with only 1 concrete type.

P.S. When using a concrete reference, just the strongly-typed API is exposed. Only when using interfaces do you lose type safety, and even then it's not lost, just 1 step away :-)

Where is Ayende?

Alex: off to see some wizard I guess

Bryan: That's right, but whenever I use a concrete type for a reference, I'm afraid some blogger like Ayende will jump out of an open browser window and drag me to the daily WTF ;-) I'd rather have a different name/signature.

Ayende,

Can you post a complete working example of your solution? With mapping messages to handlers etc.