Challengecalling generics without the generic type
Assume that I have the following interface:
public interface IMessageHandler<T> where T : AbstractMessage { void Handle(T msg); }
How would you write this method so dispatching a message doesn't require reflection every time:
public void Dispatch(AbstractMessage msg) { IMessageHandler<msg.GetType()> handler = new MyHandler<msg.GetType()>(); handler.Handle(msg); }
Note that you can use reflection the first time you encounter a message of a particular type, but not in any subsequent calls.
More posts in "Challenge" series:
- (01 Jul 2024) Efficient snapshotable state
- (13 Oct 2023) Fastest node selection metastable error state–answer
- (12 Oct 2023) Fastest node selection metastable error state
- (19 Sep 2023) Spot the bug
- (04 Jan 2023) what does this code print?
- (14 Dec 2022) What does this code print?
- (01 Jul 2022) Find the stack smash bug… – answer
- (30 Jun 2022) Find the stack smash bug…
- (03 Jun 2022) Spot the data corruption
- (06 May 2022) Spot the optimization–solution
- (05 May 2022) Spot the optimization
- (06 Apr 2022) Why is this code broken?
- (16 Dec 2021) Find the slow down–answer
- (15 Dec 2021) Find the slow down
- (03 Nov 2021) The code review bug that gives me nightmares–The fix
- (02 Nov 2021) The code review bug that gives me nightmares–the issue
- (01 Nov 2021) The code review bug that gives me nightmares
- (16 Jun 2021) Detecting livelihood in a distributed cluster
- (21 Apr 2020) Generate matching shard id–answer
- (20 Apr 2020) Generate matching shard id
- (02 Jan 2020) Spot the bug in the stream
- (28 Sep 2018) The loop that leaks–Answer
- (27 Sep 2018) The loop that leaks
- (03 Apr 2018) The invisible concurrency bug–Answer
- (02 Apr 2018) The invisible concurrency bug
- (31 Jan 2018) Find the bug in the fix–answer
- (30 Jan 2018) Find the bug in the fix
- (19 Jan 2017) What does this code do?
- (26 Jul 2016) The race condition in the TCP stack, answer
- (25 Jul 2016) The race condition in the TCP stack
- (28 Apr 2015) What is the meaning of this change?
- (26 Sep 2013) Spot the bug
- (27 May 2013) The problem of locking down tasks…
- (17 Oct 2011) Minimum number of round trips
- (23 Aug 2011) Recent Comments with Future Posts
- (02 Aug 2011) Modifying execution approaches
- (29 Apr 2011) Stop the leaks
- (23 Dec 2010) This code should never hit production
- (17 Dec 2010) Your own ThreadLocal
- (03 Dec 2010) Querying relative information with RavenDB
- (29 Jun 2010) Find the bug
- (23 Jun 2010) Dynamically dynamic
- (28 Apr 2010) What killed the application?
- (19 Mar 2010) What does this code do?
- (04 Mar 2010) Robust enumeration over external code
- (16 Feb 2010) Premature optimization, and all of that…
- (12 Feb 2010) Efficient querying
- (10 Feb 2010) Find the resource leak
- (21 Oct 2009) Can you spot the bug?
- (18 Oct 2009) Why is this wrong?
- (17 Oct 2009) Write the check in comment
- (15 Sep 2009) NH Prof Exporting Reports
- (02 Sep 2009) The lazy loaded inheritance many to one association OR/M conundrum
- (01 Sep 2009) Why isn’t select broken?
- (06 Aug 2009) Find the bug fixes
- (26 May 2009) Find the bug
- (14 May 2009) multi threaded test failure
- (11 May 2009) The regex that doesn’t match
- (24 Mar 2009) probability based selection
- (13 Mar 2009) C# Rewriting
- (18 Feb 2009) write a self extracting program
- (04 Sep 2008) Don't stop with the first DSL abstraction
- (02 Aug 2008) What is the problem?
- (28 Jul 2008) What does this code do?
- (26 Jul 2008) Find the bug fix
- (05 Jul 2008) Find the deadlock
- (03 Jul 2008) Find the bug
- (02 Jul 2008) What is wrong with this code
- (05 Jun 2008) why did the tests fail?
- (27 May 2008) Striving for better syntax
- (13 Apr 2008) calling generics without the generic type
- (12 Apr 2008) The directory tree
- (24 Mar 2008) Find the version
- (21 Jan 2008) Strongly typing weakly typed code
- (28 Jun 2007) Windsor Null Object Dependency Facility
Comments
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
{
}
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>();
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 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
{
}
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)
{
}
}
public class Dispatcher
{
private delegate void DispatchMessageDelegate(AbstractMessage msg);
private readonly Dictionary<Type, DispatchMessageDelegate> dispatchers = new Dictionary<Type, DispatchMessageDelegate>();
public void Dispatch(AbstractMessage msg)
{
}
private DispatchMessageDelegate GetDispatchDelegate(Type messageType)
{
}
private static DispatchMessageDelegate GenerateDispatchDelegate(Type messageType)
{
}
}
[TestFixture]
public class TestHandler
{
[Test]
public void ShouldWork()
{
}
}
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>() {
}
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
{
}
public interface IMessageHandler<T> : IMessageHandler where T : AbstractMessage
{
}
public class MessageHandler<T> : IMessageHandler<T>
{
}
public static class Messaging
{
}
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
{
}
That means that the concrete implementation could look like this:
internal class MyHandler : IMessageHandler
{
}
And the calling code:
public class Caller
{
}
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,
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> {
}}
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)
{
}
else if(msg is Msg2)
{
}
else
{
}
}
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?
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)
{
}
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:
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.
Comment preview