Architecturally speaking, how should I replace the extremely large switch statement with something more manageable?

Question

EDIT 1: Forgot to add a ball to the curve of the attached property.

UPDATE: I chose @mtazva's answer as this was the preferred solution for my particular case. Looking back, I asked a general question with a very concrete example, and I believe that in the end everyone was confused (or maybe just me) as regards the question. I believe that a general question was also answered (see Answers and links to the strategy template). Thanks everyone!

The operators of a large operator are obviously smell , and I saw some links to how you could do this using a dictionary that maps to functions . But I wonder if there is a better (or smarter way) to do this? In a way, this is a question that I always somehow rolled down the back of my head, but really did not have a good solution.

This question arose from another question that I asked earlier: How to select all values ​​of an object property in a list of typed objects in .Net with C #

Here is an example of the class I'm working with (from an external source):

public class NestedGameInfoObject { public string NestedName { get; set; } public int NestedIntValue { get; set; } public decimal NestedDecimalValue { get; set; } } public class GameInfo { public int UserId { get; set; } public int MatchesWon { get; set; } public long BulletsFired { get; set; } public string LastLevelVisited { get; set; } public NestedGameInfoObject SuperCoolNestedGameInfo { get; set; } // thousands more of these } 

Unfortunately, this is due to an external source ... imagine that HUGE dump data from Grand Theft Auto or something like that.

And I want to get only a small cross section of the list of these objects. Imagine that we want to compare you with a bunch of objects of information about the game of your friends. An individual result for one user will look like this:

 public class MyResult { public int UserId { get; set; } // user id from above object public string ResultValue { get; set; } // one of the value fields from above with .ToString() executed on it } 

And an example of what I want to replace with something more manageable (believe me, I DO NOT want to support this monster switch statement):

 const int MATCHES_WON = 1; const int BULLETS_FIRED = 2; const int NESTED_INT = 3; public static List<MyResult> GetMyResult(GameInfo[] gameInfos, int input) { var output = new List<MyResult>(); switch(input) { case MATCHES_WON: output = gameInfos.Select(x => new MyResult() { UserId = x.UserId, ResultValue = x.MatchesWon.ToString() }).ToList<MyResult>(); break; case BULLETS_FIRED: output = gameInfos.Select(x => new MyResult() { UserId = x.UserId, ResultValue = x.BulletsFired.ToString() }).ToList<MyResult>(); break; case NESTED_INT: output = gameInfos.Select(x => new MyResult() { UserId = x.UserId, ResultValue = x.SuperCoolNestedGameInfo.NestedIntValue.ToString() }).ToList<MyResult>(); break; // ad nauseum } return output; } 

So the question is, are there any reasonable ways to control this beast? I would really like the dynamic way to get this information in case the initial changes to the object (for example, additional properties of the game information are added). Is there a better way to architect this so that it is less awkward?

Answer 1

I think your first sentence has eluded what is probably the most sensible solution: some form of dictionary mapping values ​​for methods.

For example, you can define a static Dictionary<int, func<GameInfo, string>> where each value, such as MATCHES_WON, will be added with a corresponding lambda that retrieves the corresponding value (provided that your constants, etc. are defined, as shown in your example):

 private static Dictionary<int, Func<GameInfo, string>> valueExtractors = new Dictionary<int, Func<GameInfo, string>>() { {MATCHES_WON, gi => gi.MatchesWon.ToString()}, {BULLETS_FIRED, gi => gi.BulletsFired.ToString()}, //.... etc for all value extractions }; 

Then you can use this dictionary to retrieve the value in your example:

 public static List<MyResult> GetMyResult(GameInfo[] gameInfos, int input) { return gameInfo.Select(gi => new MyResult() { UserId = gi.UserId, ResultValue = valueExtractors[input](gi) }).ToList<MyResult>(); } 

Outside of this option, you can have some kind of file / database / saved search with the property number and name, and then use reflection to retrieve the value, but this obviously will not be done.

Answer 2

I think this code is a little out of control. You effectively use constants to index properties - and this creates fragile code that you want to use with some technique - for example, reflection, dictionaries, etc. - to manage increased complexity.

Effectively, the approach you are using now will have this code:

 var results = GetMyResult(gameInfos, BULLETS_FIRED); 

An alternative is to define an extension method that allows you to do this:

 var results = gameInfos.ToMyResults(gi => gi.BulletsFired); 

This is strongly typed; it does not require constants, switch statements, reflection, or anything secret.

Just write these extension methods and you're done:

 public static class GameInfoEx { public static IEnumerable<MyResult> ToMyResults( this IEnumerable<GameInfo> gameInfos, Func<GameInfo, object> selector) { return gameInfos.Select(gi => gi.ToMyResult(selector)); } public static MyResult ToMyResult( this GameInfo gameInfo, Func<GameInfo, object> selector) { return new MyResult() { UserId = gameInfo.UserId, ResultValue = selector(gameInfo).ToString() }; } } 

Does this work for you?

Answer 3

You can use reflection for abstract purposes. You can implement custom attributes, mark your properties, etc. It is also a dynamic way to get information about your class if it changes.

Answer 4

If you want to manage the switch code, I would point you to the Design Patterns (GoF) book and suggest that I might consider patterns such as Strategy and possibly Factory (thats, when we talk about use in general, your case isn ' t is very suitable for Factory) and their implementation.

While the switch statement should still be left somewhere after refactoring to the template (for example, in the place where you select the strategy by id), the code will be much smoother and more understandable.

As for the general maintenance of the switch, if they become like an beast, I'm not sure if this is the best solution, given how similar statements in your case look.

I am 100% sure that you can create some method (possibly an extension method) that will take the desired lambda properties accessory that should be used when creating the results.

Answer 5

If you want your code to be more general, I agree with the suggestion of a dictionary or some kind of search template.

You can store functions in a dictionary, but it seems that they all perform the same operation - getting the value from the property. It is ripe for reflection.

I would save all your properties in a dictionary with an enumeration (I prefer enumeration to const ) as a key, and PropertyInfo - or, less preferably, a string that describes the name of the property as Value. Then you call the GetValue() method of the PropertyInfo object to retrieve the value from the object / class.

Here is an example when I map enum values ​​to their “same” properties in a class, and then use reflection to extract values ​​from the class.

 public enum Properties { A, B } public class Test { public string A { get; set; } public int B { get; set; } } static void Main() { var test = new Test() { A = "A value", B = 100 }; var lookup = new Dictionary<Properties, System.Reflection.PropertyInfo>(); var properties = typeof(Test).GetProperties().ToList(); foreach (var property in properties) { Properties propertyKey; if (Enum.TryParse(property.Name, out propertyKey)) { lookup.Add(propertyKey, property); } } Console.WriteLine("A is " + lookup[Properties.A].GetValue(test, null)); Console.WriteLine("B is " + lookup[Properties.B].GetValue(test, null)); } 

You can match your const values ​​with property names, PropertyInfo objects that relate to these properties, functions that will retrieve property values ​​... whatever you think according to your needs.

Of course, you will need some mapping - somewhere along the way you will depend on your input value ( const ) for a particular property. The method by which you can obtain this data can determine the best structure and display structure for you.

Answer 6

I think the way to go is really some kind of mapping from a single value (int) to something that is somehow a function that knows how to extract the value. If you really want to keep it extensible so that you can easily add it without touching the code and possibly access to more complex properties (i.e., Nested properties, perform some basic calculations), you may want to save this in a separate source.

I think one way to do this is to rely on Scripting Services, for example, evaluating a simple IronPython expression to extract a value ...

For example, in a file you can store something like:

 <GameStats> <GameStat name="MatchesWon" id="1"> <Expression> currentGameInfo.BulletsFired.ToString() </Expression> </GameStat> <GameStat name="FancyStat" id="2"> <Expression> currentGameInfo.SuperCoolNestedGameInfo.NestedIntValue.ToString() </Expression> </GameStat> </GameStats> 

and then, depending on the requested stat, you always return the generic GameInfos. You can have some kind of foreach loop with:

 foreach( var gameInfo in gameInfos){ var currentGameInfo = gameInfo //evaluate the expression for this currentGameInfo return yield resultOfEvaluation } 

See http://www.voidspace.org.uk/ironpython/dlr_hosting.shtml for examples of how to embed an IronPython script in a .NET application.

NOTE. When dealing with such things, there are several things you should really be careful about:

• this potentially allows someone to enter code into your application ...
• you must measure the impact of dynamic evaluation on performance

Answer 7

I do not have a solution to the problem with the switch, but of course you can reduce the code using a class that can automatically display all the fields you need. Check out http://automapper.org/ .

Answer 8

I would not write the GetMyResult method in the first place. All he does is convert a GameInfo sequence to a MyResult sequence. Doing this with Linq would be simpler and more expressive.

Instead of calling

 var myResultSequence = GetMyResult(gameInfo, MatchesWon); 

I would just call

 var myResultSequence = gameInfo.Select(x => new MyResult() { UserId = x.UserId, ResultValue = x.MatchesWon.ToString() }); 

To make it more concise, you can pass UserId and ResultValue in the constructor

  var myResultSequence = gameInfo.Select(x => new MyResult(x.UserId, x.MatchesWon.ToString())); 

Refactoring only if you see that it selects too many duplicates.

Answer 9

This is one of the possible ways without using reflection:

 using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace ConsoleApplication1 { public class GameInfo { public int UserId { get; set; } public int MatchesWon { get; set; } public long BulletsFired { get; set; } public string LastLevelVisited { get; set; } // thousands more of these } public class MyResult { public int UserId { get; set; } // user id from above object public string ResultValue { get; set; } // one of the value fields from above with .ToString() executed on it } public enum DataType { MatchesWon = 1, BulletsFired = 2, // add more as needed } class Program { private static Dictionary<DataType, Func<GameInfo, object>> getDataFuncs = new Dictionary<DataType, Func<GameInfo, object>> { { DataType.MatchesWon, info => info.MatchesWon }, { DataType.BulletsFired, info => info.BulletsFired }, // add more as needed }; public static IEnumerable<MyResult> GetMyResult(GameInfo[] gameInfos, DataType input) { var getDataFunc = getDataFuncs[input]; return gameInfos.Select(info => new MyResult() { UserId = info.UserId, ResultValue = getDataFunc(info).ToString() }); } static void Main(string[] args) { var testData = new GameInfo[] { new GameInfo { UserId="a", BulletsFired = 99, MatchesWon = 2 }, new GameInfo { UserId="b", BulletsFired = 0, MatchesWon = 0 }, }; // you can now easily select whatever data you need, in a type-safe manner var dataToGet = DataType.MatchesWon; var results = GetMyResult(testData, dataToGet); } } } 

Answer 10

Purely on the question of large switch statements, it is noteworthy that there are two variants of the Cyclomatic Complexity metric in general use. The "original" counts each case statement as a branch and therefore increases the complexity metric by 1, which leads to a very high value caused by many switches. The "variant" calculates the switch statement as a single branch - this effectively considers it as a sequence of non-branching statements, which is more consistent with the goal of "understanding" complexity control.