Today I found myself writing this piece of code:
switch (searchType)
{
case SearchType.Bananas:
SearchForBananas();
break;
case SearchType.Oranges:
SearchForOranges();
break;
case SearchType.Kiwis:
SearchForKiwis();
break;
default:
SearchForBananas();
SearchForOranges();
SearchForKiwis();
}
Except for me not being very found of switch statements this code might look OK at first glance, but it has one serious flaw. What happens if I, or someone else, have to add another search method? Or remove one of the methods? We have to make changes in two places. First we have to add another case and then we have to add one more method call to the default case. In my opinion that severely reduces the flexibility (or agility if you will) of the code and increases the risk of bugs being introduced when changes are made, especially when there are a lot more actions than three.
Still, I often find myself writing code similar to this and while I know there are at least one better solution, an action map, or the “delegate dictionary” pattern if you will, I keep forgetting how to do it. Hopefully blogging about the problem will fix that :-)
Anyway, using a delegate dictionary the code can be refactored to:
Dictionary<SearchType, Action> actionMap = new Dictionary<SearchType, Action>();
actionMap.Add(SearchType.Bananas, SearchForBananas);
actionMap.Add(SearchType.Oranges, SearchForOranges);
actionMap.Add(SearchType.Kiwis, SearchForKiwis);
if(actionMap.ContainsKey(searchType))
actionMap[searchType]();
else
foreach (Action action in actionMap.Values)
action();
While the amount of code is only slightly reduced with this approach it makes the code much more flexible as adding or removing actions only requires the code to be changed in a single place. And when there’s 15 possible actions instead of three it also looks a hell of a lot prettier.
PS. For updates about new posts, sites I find useful and the occasional rant you can follow me on Twitter. You are also most welcome to subscribe to the RSS-feed.
This blog is built with EPiServer Community, EPiServer CMS, ASP.NET MVC and a bunch of other great products. The source code is available for download at the projects page, where you also can read more about this site and my other projects.
read more
Comments
Frederik Vig 9 months ago
Same approach as you with the Dictionary collection, but still good read: http://elegantcode.com/2009/01/10/refactoring-a-switch-statement/ and http://elegantcode.com/2009/01/31/refactoring-the-refactored-switch-statement/.
Martin S. 9 months ago
Thank you for sharing!
It's very common to find code like that in any problem, and the solution looks just classy.
Keep 'em coming Joel!
Johan Pettersson 9 months ago
Nice one!
Kalle Hoppe 9 months ago
Yeah, this is a much cleaner and nicer way of doing the same thing. The thing I like most is the part when you only have to do one check, if it exists, execute!
Dan Jansson 9 months ago
Nice post! Currently I'm experimenting a lot with generics and delegates and I just love it and think it's neat! I will try out your example a.s.a.p.
Jeff Doolittle 8 months ago
I find myself doing a lot of this as well, but there is one thing I still don't like about the dictionary implementation you've posted. I still have to remember to register my delegates (actionMap.Add()).
What I've done lately is to create an interface and then set up my concrete class to receive an array of that interface type in its constructor. Then I set up my IoC container (StructureMap) to find all instances which implement the interface and inject them into my concrete class. This way, when I create a new implementation of the interface, it will auto-magically be included in the array parameter passed to my concrete class!
This may be overkill when you have only three search types. But as the number grows, it becomes much easier to manage each of these in their own class file.
If you want to try this out, just cut and past the code below into a new console application .cs file (also make sure your new project has a reference to StructureMap).
So, for example:
public class Fruit { // fruit properties and other implementation would go here } public enum SearchType { Bananas, Oranges, Kiwis } public interface IFruitSearch { bool ShouldApply(SearchType searchType); IEnumerable Search(); } public class BananaSearch : IFruitSearch { public bool ShouldApply(SearchType searchType) { return searchType == SearchType.Bananas; } public IEnumerable Search() { // search for bananas return new Fruit[] { }; // you would actually search for and return fruit here } } public class OrangeSearch : IFruitSearch { public bool ShouldApply(SearchType searchType) { return searchType == SearchType.Oranges; } public IEnumerable Search() { // search for oranges return new Fruit[] { }; // you would actually search for and return fruit here } } public class KiwiSearch : IFruitSearch { public bool ShouldApply(SearchType searchType) { return searchType == SearchType.Oranges; } public IEnumerable Search() { // search for Kiwis return new Fruit[] { }; // you would actually search for and return fruit here } } public class FruitFinder { private readonly IFruitSearch[] _filters; public FruitFinder(IFruitSearch[] filters) { _filters = filters; } public IEnumerable GetFruit(SearchType searchType) { foreach (var filter in _filters) { if (filter.ShouldApply(searchType)) return filter.Search(); } return new Fruit[] { }; // you would provide a fall through method here if no applicable filter was found } public IEnumerable Filters // only exposed to demonstrate that filters are being properly injected from StructureMap { get { return _filters; } } } public static class Program { public static void Bootstrap() { ObjectFactory.Configure( x => x.Scan( s => { s.AssemblyContainingType(); s.WithDefaultConventions(); s.AddAllTypesOf(); } )); } public static void Main() { Bootstrap(); var finder = ObjectFactory.GetInstance(); Console.WriteLine(string.Format("Resolved {0} filters for FruitFinder", finder.Filters.Count())); Console.ReadKey(); } }Jeff Doolittle 8 months ago
Sorry, forgot to replace my angle brackets in that last comment. Let me know if you want me to repost.
Natural Makeup 8 months ago
Nice Code mate and nice guide. I will find these very useful in my school homework's, where we are quite often using C++.
Also I wish a great year 2010 to you.
Visual C# Kicks 7 months ago
Pretty cool approach. Although I'm intrigued by the method Jeff described too.
N.Stronge 2 days ago
Anabolic Steroid Online Shop allanabolics.com has been designed to reply all questions regarding anabolic steroids and for fast and convenient purchase all known anabolic steroids. buy anabolics online
Special Prices And Discounts We offer special prices and discounts for all large quantity purchases.