design for a method with cyclomatic complexity issue
-
I have a conceptually simple method that gets a compiler warning (CA1502) regarding its cyclomatic complexity. The error tells me that the method has a complexity of 31 and I should reduce that to 25. The problem is that the method isn't doing much, and the complexity is due to a switch block that maps an old DAO data type enumeration to a .NET type. For example, int for adInteger. The only thing else the code does is parse the input string into an integer. Overall, this method is part of a builder pattern based class that is responsible for taking a formatted string and converting it into a DataSet. The data comes through TCP from a legacy application that's output is based on a DAO Recordset. At this point, I'm settling on ignoring the compiler warning, because the other options I've considered have consequences that outway the benefits. For example, implementing a look-up such as Dictionary takes up memory unnecessarily. This method will either be called very rarely or very often. Before I walk away from this, does anyone have any ideas for alternatives to this? I appreciate any advice. Here is the code in question (I've removed some of the data types for brevity):
private static Type GetColumnDataType(string data) { int typeValue; bool parsed = int.TryParse(data, out typeValue); if (!parsed) throw new FormatException(Properties.Resources.BuilderColumnFormatMismatch); DAOColumnDataType dataType = (DAOColumnDataType)typeValue; Type resolvedType; switch (dataType) { case DAOColumnDataType.adVarChar: resolvedType = typeof(string); break; case DAOColumnDataType.adBoolean: resolvedType = typeof(bool); break; case DAOColumnDataType.adInteger: &n -
I have a conceptually simple method that gets a compiler warning (CA1502) regarding its cyclomatic complexity. The error tells me that the method has a complexity of 31 and I should reduce that to 25. The problem is that the method isn't doing much, and the complexity is due to a switch block that maps an old DAO data type enumeration to a .NET type. For example, int for adInteger. The only thing else the code does is parse the input string into an integer. Overall, this method is part of a builder pattern based class that is responsible for taking a formatted string and converting it into a DataSet. The data comes through TCP from a legacy application that's output is based on a DAO Recordset. At this point, I'm settling on ignoring the compiler warning, because the other options I've considered have consequences that outway the benefits. For example, implementing a look-up such as Dictionary takes up memory unnecessarily. This method will either be called very rarely or very often. Before I walk away from this, does anyone have any ideas for alternatives to this? I appreciate any advice. Here is the code in question (I've removed some of the data types for brevity):
private static Type GetColumnDataType(string data) { int typeValue; bool parsed = int.TryParse(data, out typeValue); if (!parsed) throw new FormatException(Properties.Resources.BuilderColumnFormatMismatch); DAOColumnDataType dataType = (DAOColumnDataType)typeValue; Type resolvedType; switch (dataType) { case DAOColumnDataType.adVarChar: resolvedType = typeof(string); break; case DAOColumnDataType.adBoolean: resolvedType = typeof(bool); break; case DAOColumnDataType.adInteger: &nHi, you could use a HashTable to map all the DAOColumnDataType values to their corresponding types. The table needs to be constructed only once. If the key exists, the value is the type; if not (which corresponds to the default case in your switch), typeof(string) is what you want. Furthermore, if all DAOColumnDataType.ad### constants are integers within a small range, you could even consider an array of types and use dataType as the index (probably with a try-catch to catch excessive index values); any intermediate non-existing ad### values should result in typeof(string). Hope this helps :)
Luc Pattyn [My Articles] [Forum Guidelines]
-
Hi, you could use a HashTable to map all the DAOColumnDataType values to their corresponding types. The table needs to be constructed only once. If the key exists, the value is the type; if not (which corresponds to the default case in your switch), typeof(string) is what you want. Furthermore, if all DAOColumnDataType.ad### constants are integers within a small range, you could even consider an array of types and use dataType as the index (probably with a try-catch to catch excessive index values); any intermediate non-existing ad### values should result in typeof(string). Hope this helps :)
Luc Pattyn [My Articles] [Forum Guidelines]
That was kind of what I was going for with the generic Dictionary (Dictionary). It just sucks up a bit of memory that I couldn't justify since the method may only be called rarely (the other scenario is that it would be used constantly). Some of the items are continuous in their values specifically 2-7, 11, 14, 16, 20, 72, 129, 131, 133-135, and 200 are the ones handled. Doesn't seem to be any pattern there to me. Although an array would suck up the memory too. I'd probably do that in a static implementation; but, wouldn't matter anyway since System.Type objects are never garbage collected.
-
That was kind of what I was going for with the generic Dictionary (Dictionary). It just sucks up a bit of memory that I couldn't justify since the method may only be called rarely (the other scenario is that it would be used constantly). Some of the items are continuous in their values specifically 2-7, 11, 14, 16, 20, 72, 129, 131, 133-135, and 200 are the ones handled. Doesn't seem to be any pattern there to me. Although an array would suck up the memory too. I'd probably do that in a static implementation; but, wouldn't matter anyway since System.Type objects are never garbage collected.
tgrt wrote:
System.Type objects are never garbage collected
I dont know; seems to make sense tho. If so, an array approach only costs 4 (or 8) B per entry, since the types exist whatever approach you take. Hence less than 1KB. Way to go ! :)
Luc Pattyn [My Articles] [Forum Guidelines]
-
tgrt wrote:
System.Type objects are never garbage collected
I dont know; seems to make sense tho. If so, an array approach only costs 4 (or 8) B per entry, since the types exist whatever approach you take. Hence less than 1KB. Way to go ! :)
Luc Pattyn [My Articles] [Forum Guidelines]
-
Yeah, I guess that might work the Dictionary/Hashtable approach. An array wouldn't work, because of the large spread of values. I'm creating less than 20 System.Type objects. I guess I should factor in the smaller amount of code loaded into memory.
I would go for the array, something like:
Type[] daTypes=new Type[256];
...
// initialize
for(int i=0; i<256; i++) daTypes[i]=typeof(string);
daTypes[daConstant]=typeof(something); // repeat for each daConstant
...// use
if ((val & 255)!=val) {
type=typeof(string); // outside range [0, 255]
} else {
type=daTypes[val]; // inside range [0, 255]
}so there is even no need to try-catch; only 1KB of data, and much faster than switch or HashTable. :)
Luc Pattyn [My Articles] [Forum Guidelines]