 |
|
|
 |
|
|
 |
|
|
 |
|
 |
the code is nice and clean, I like it. 5 stars!
|
|
|
|
 |
|
 |
1) More typically, you should use anonymous methods for delegates (arguments passed to add).
2) You don't need to make delegate type a generic type argument; instead, you could make the delegate type itself generic (with type argument 'enum') and add one 'enum' argument passed in the delegate call (or you can combine both approaches); the 'enum' value is useful information for implementation of the delegate.
You can even use the same delegate implementation for some subset of key values if you need similar but slightly different processing for different keys in the same delegate implementation. One ridiculus way of using this would be creating a shorter swith operator inside a delegate implementation! (No, I don't advise doing this in practice...)
Sergey A Kryukov
|
|
|
|
 |
|
 |
I confirm by my experience that this approach is quite correct.
Event though you slightly add to memory footprint, you win performance, not only convenience and supportability.
My 5.
I would advise one improvement: add one more method accepting the delegate argument: something like SetDefaultAction to cover all values not found in the dictionary. Initially, this default action should be null.
Thank you.
Sergey A Kryukov
|
|
|
|
 |
|
 |
In switch statement you can call functions which may call other functions as well. I like delegate idea in general with use of dictionary, but not sure if it simplify the switch statement.
switch(x){
case a:
A();
break;
case b:
B();
break;
...
}
void A(){
X();
Y();
};
void B(){
X();
....
}
|
|
|
|
 |
|
 |
I agree with this, you can call functions from the switch. this is what I do,
|
|
|
|
 |
|
 |
Timmy,
You've come up with a very elegant solution! I am curious though; what kind of speed difference is there between this solution and the switch blocks? Obviously its very minimal BUT do this a million times and the difference may be more than trivial. Do you have any idea on this?
|
|
|
|
 |
|
 |
If you look at the comment of Jamie Nordmeyer. He has taken a look at the amount of IL produced in both cases. This is about the same. I havn't been able to do some speed tests. I will, but I do not have the time at the moment.
Dawn is nature's way of telling you to go to bed.
|
|
|
|
 |
|
 |
Timmy, the amount if IL code has little to do with actual performance.
My prediction is that your solution will usually give better performance if you compare your implementation with switch-based implementation with the the number of keys (create by your sequence of add methods) being the same as the number of switch cases; more exactly, your solution will be faster if this number is big enough.
Design of dictionaries is based on hash values and -- as I understand it -- backets. This design provides speed much better than linear: close to O(1), according to Microsoft documentation. As switch can only provide linear (O(n)) speed, so your gain in speed compared to switch will grow as linear function with number of cases to be processed; your speed will be close to some constant time span regardless of the number of cases.
Sergey A Kryukov
|
|
|
|
 |
|
 |
Really you provided a good solution to reduce complexity, but I think one of the good benifits of it is the idea of using objects in the switch statement instead of integers and enums, this way we can use any objects we want.
Sincerely Samer Abu Rabie
Note: Please remember to rate this post to help others whom reading it.
|
|
|
|
 |
|
 |
Hi, Timmy
You've made my day 2d easy for what i want.
Good article.
|
|
|
|
 |
|
 |
I was writing a program to match code in abstract syntax trees. Basically parsed C# code. This leads to a bunch of small methods that specifically match one statement/expression to another. For example two assignment statements. I made a static class that had a method for each statement/expression type and used reflection to build the dictionary that cross referenced the statement/expression type to the method that dealt with it. My methods looked like:
public static void MatchAssignmentStatement(AssignmentStatement left, AssignmentStatement right) {...}
I then used Linq.Expression to build and compile a delegate appropriate for each. This gets you away from having to hard code building the dictionary.
Another different but similar thing I've done is use the text templating engine built into Visual Studio 2008. These are files with the .tt extention. I would use reflection in the template itself to build a C# class. In your case, you could use the enum to build the code to populate your dictionary.
OOPS, the wife is home. Let me know if you are interested...
|
|
|
|
 |
|
 |
Great ideas! Could you send me an example of your code? I'm very curious how you did this.
Dawn is nature's way of telling you to go to bed.
|
|
|
|
 |
|
 |
The following code is from my project on CodePlex at http://www.codeplex.com/JSLRefactorExplorer. It's in the MatchMethodLookup.cs file. I will probably be cleaning and refactoring it sometime in the future but right now I've got other things to do. All in all the codes pretty simple if you understand reflection, Linq and Linq expressions. The project is in its very early stages. I'm planning on making big changes which will change this code slightly.
The objective of the code is to build a dictionary with a Type as the key and a delegate as the value. The Type that I'm passing in has a bunch of static methods starting who's names start with "Match" and take three parameters. The important parameters are the second and third ones. They have to be the same and derive off of CodeUnit or CsElement. All that is the WHERE part of the Linq query in the BuildLookupTable(Type type) method.
The return statement takes the Linq query and builds a dictionary out of it. That's about it for that method.
The CreateDelegate method takes the MethodInfo found in the above method and creates a delegate that's defined in another file in the project:
public delegate void MatcherMethod(MatchState state, ICodeUnit left, ICodeUnit right);
The method builds the three parameters for the delegate, adds a single statement which is a method call and compiles the expression. The second and third parameter types of the methods being called implement ICodeUnit. For example AssignementStatement is an Abstract Syntax Tree element that implements ICodeUnit. Since the methods being called by my Linq expression are "more specific", the Linq expression has to take in two ICodeUnit's and convert them to the appropriate concrete types. Your example was working with enums so you wouldn't have to do this step.
So to wrap things up... The performance hit from the reflection and the Linq compiling is very small. Best of all, I only have to add a new "Match" method to the appropriate class and it automatically gets use! There's no need to add code to add a new item to a dictionary, etc.
Here's the code from my CodePlex project. It will probably change in the next couple days but the process will be similar.
namespace Jsl.RefactoringExplorer
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.StyleCop.CSharp;
using Expression = System.Linq.Expressions.Expression;
public static class MatchMethodLookup
{
#region Public Methods
public static IDictionary<type,> BuildLookupTable(Type type)
{
var methods =
from method in type.GetMethods(BindingFlags.Public | BindingFlags.Static)
let parameters = method.GetParameters()
where method.Name.StartsWith("Match", StringComparison.Ordinal)
&& parameters.Length == 3
&& parameters[0].ParameterType == typeof(MatchState)
&& (parameters[1].ParameterType.IsSubclassOf(typeof(CodeUnit))
|| parameters[1].ParameterType.IsSubclassOf(typeof(CsElement)))
&& parameters[2].ParameterType == parameters[1].ParameterType
select new { method, parameters[1].ParameterType };
return methods.ToDictionary(
methodInfo => methodInfo.ParameterType,
methodInfo => MatchMethodLookup.CreateDelegate(methodInfo.method, methodInfo.ParameterType));
}
#endregion Public Methods
#region Private Methods
private static MatchState.MatcherMethod CreateDelegate(MethodInfo method, Type parameterType)
{
var matchStateParameter = Expression.Parameter(typeof(MatchState), "state");
var leftParameter = Expression.Parameter(typeof(ICodeUnit), "left");
var rightParameter = Expression.Parameter(typeof(ICodeUnit), "right");
var leftAsSpecific = Expression.Convert(leftParameter, parameterType);
var rightAsSpecific = Expression.Convert(rightParameter, parameterType);
var methodCallExpression = Expression.Call(method, matchStateParameter, leftAsSpecific, rightAsSpecific);
return Expression.Lambda<matchstate.matchermethod>(
methodCallExpression,
matchStateParameter,
leftParameter,
rightParameter).Compile();
}
#endregion Private Methods
}
}</matchstate.matchermethod>
|
|
|
|
 |
|
|
 |
|
 |
I've actually done this in a couple of production apps, and it worked great! Much easier to read. I'd be interested in running a test case at some point to see how this method compares to a switch statement in the generated IL, though.
Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan
Portland, Oregon, USA
|
|
|
|
 |
|
 |
That's what I thought too. Are you using unit tests too? If you test this constuction I'd be very much interested in the results. I do not have time to run a test case in the next few weeks, maybe after that.
Dawn is nature's way of telling you to go to bed.
|
|
|
|
 |
|
 |
So, it looks like the switch statement produces about the same amount of IL. The dictionary method is still nice, as it allows for dynamic control of the "switching" at run time. I used the following code:
using System;
using System.Collections.Generic;
namespace App
{
public enum Color
{
Red,
Blue,
Yellow
}
public class Tester
{
private delegate void Method();
private static Dictionary<Color,Method> _delegates;
public static void UseSwitch(Color color)
{
switch (color)
{
case Color.Red:
Console.WriteLine("Eat a raspberry");
break;
case Color.Blue:
Console.WriteLine("Eat a blueberry");
break;
case Color.Yellow:
Console.WriteLine("Eat a banana");
break;
}
}
public static void UseDictionary(Color color)
{
_delegates[color]();
}
public static void UseRed()
{
Console.WriteLine("Eat a raspberry");
}
public static void UseBlue()
{
Console.WriteLine("Eat a blueberry");
}
public static void UseYellow()
{
Console.WriteLine("Eat a banana");
}
public static void Main()
{
_delegates = new Dictionary<Color,Method>();
_delegates.Add(Color.Red, UseRed);
_delegates.Add(Color.Blue, UseBlue);
_delegates.Add(Color.Yellow, UseYellow);
UseSwitch(Color.Red);
UseDictionary(Color.Yellow);
}
}
}
The IL for the switch statement looks like this (14 lines of IL):
.method public hidebysig static void UseSwitch(valuetype App.Color color) cil managed
{
.maxstack 1
.locals init (
[0] valuetype App.Color color2)
L_0000: ldarg.0
L_0001: stloc.0
L_0002: ldloc.0
L_0003: switch (L_0015, L_0020, L_002b)
L_0014: ret
L_0015: ldstr "Eat a raspberry"
L_001a: call void [mscorlib]System.Console::WriteLine(string)
L_001f: ret
L_0020: ldstr "Eat a blueberry"
L_0025: call void [mscorlib]System.Console::WriteLine(string)
L_002a: ret
L_002b: ldstr "Eat a banana"
L_0030: call void [mscorlib]System.Console::WriteLine(string)
L_0035: ret
}
The IL for the dictionary call looks like so :
.method public hidebysig static void UseDictionary(valuetype App.Color color) cil managed
{
.maxstack 8
L_0000: ldsfld class [mscorlib]System.Collections.Generic.Dictionary`2 App.Tester::_delegates
L_0005: ldarg.0
L_0006: callvirt instance !1 [mscorlib]System.Collections.Generic.Dictionary`2::get_Item(!0)
L_000b: callvirt instance void App.Tester/Method::Invoke()
L_0010: ret
}
plus an additional 3 lines for each method:
.method public hidebysig static void UseRed() cil managed
{
.maxstack 8
L_0000: ldstr "Eat a raspberry"
L_0005: call void [mscorlib]System.Console::WriteLine(string)
L_000a: ret
}
My pattern for this has been to use the switch statement when I'm only comparing a few things, but to use a dictionary when I need dynamic control of the structure, or have a lot of conditionals (as a switch statement with 20 cases is incredibly ugly).
Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan
Portland, Oregon, USA
|
|
|
|
 |
|
 |
class Program
{
public enum Color
{
Red,
Blue,
Yellow
}
static string[] colors = new string[] { "Eat a raspberry", "Eat a blueberry", "Eat a bannana" };
public static void Main(string[] args)
{
Console.WriteLine(colors[(int)Color.Blue]);
}
}
The main IL assembly looks like this:
IL_0000: ldsfld string[] ConsoleApplication1.Program::colors
IL_0005: ldc.i4.1
IL_0006: ldelem.ref
IL_0007: call void [mscorlib]System.Console::WriteLine(string)
IL_000c: ret
What do I win?!
|
|
|
|
 |
|
 |
I like the way you did this. The first thing that came to mind was to create a dynamic "switch" using reflection to pull in methods at design time. I havn't put many brain cells towards (and I haven't had enough coffee to do that yet), but I think might be worth checking out.
|
|
|
|
 |
|
 |
Hello,
I like the elegance of your solution. However there is also another approach to your described problem. It is known as the "Chain of command" design pattern.
http://www.cs.clemson.edu/~malloy/courses/patterns/chain.html
The beauty of this design pattern lies in the fact that every class in the chain can decide if it is capable of handling the call or pass it on to it's successor.
Cheers.
|
|
|
|
 |
|
 |
Thanks for the tip! I'll look in to it.
Dawn is nature's way of telling you to go to bed.
|
|
|
|
 |
|
 |
But one must also know that Design patterns are not solution to a problem but rather an alternative to solve the problem easily and maintain it further. So if there is a simple solution with out design patterns, i guess it should be taken. 2 much of design pattern would kill us.
|
|
|
|
 |