Can my string to instance construction be improved?

Question

I have a Dictionary<string, object> (named tableDict) that has been parsed from JSON. The string describes the intended construction of a class T which is defined within the housing class of this code. The object is the numerical probability value that pairs up with the generated reference of T.

The string can be in either of two formats. Something encased in brackets, i.e. "(arg0, arg1, arg2, ...)", will call the constructor with these arguments. Anything else is assumed to be a name of a property of type T that returns a predefined T, i.e when T is of type Color, "Black" would lead to looking for Color.Black.

I managed to get something to work however I pieced this together from googling each problem I encountered. A lot of what I have used is new to me, it's highly likely I have gone about it in an inefficient roundabout fashion.

for (int i = 0; i < tableDict.Count; i++)
{
 KeyValuePair<string, object> kvp = tableDict.ElementAt(i);
 string s = kvp.Key;
 if (Regex.IsMatch(s, @"\(([^\)]+)\)", RegexOptions.None)) 
 // Key is "( something )" - entry is constructor
 {
 string sCon = s.WhitespaceRemoved();
 sCon = sCon.Substring(1, sCon.Length - 2); // Remove brackets 
 string[] argsAsStrings = sCon.Split(',');
 object[] args = argsAsStrings.Select(num => (object)int.Parse(num)).ToArray();
 // I have only implemented this for the Color class.
 // I assume the constructor is comprised of ints.
 T item = (T)Activator.CreateInstance(typeof(T), args);
 double prob = Convert.ToDouble(kvp.Value);
 var entry = new RawChanceTableEntry<T>(item, prob);
 list.Add(entry);
 }
 else // Parse string as a property of T instead
 {
 string sProp = s;
 Type type = typeof(T);
 PropertyInfo propInfo = type.GetProperty(sProp);
 object obj = Activator.CreateInstance(type);
 T item = (T)propInfo.GetValue(obj, null);
 double prob = Convert.ToDouble(kvp.Value);
 var entry = new RawChanceTableEntry<T>(item, prob);
 list.Add(entry);
 }
}

Note the struct RawChanceTableEntry is just a container for the item / probability pair.

Code in class: http://pastebin.com/bvJaS8Se (class is incomplete)

Parsed JSON: http://pastebin.com/HS6eSABQ

• jessehouwing: So why would you want to do this?
• Joe Shanahan: @jessehouwing I don't know how to to respond to that in any other way than "because I do". I don't understand your question.
• jessehouwing: Well you could actually send the parameters as a json object with actual ints and strings... Makes the parsing a lot easier. You could even name these the same name as the constructor expects for easier matching...
• Joe Shanahan: @jessehouwing The JSON data files are created manually, not with code.
• jessehouwing: There's no reason you couldn't hand-write easier to parse json...

Answer 1

I'd compact the main loop as such:

 for (var i = 0; i < tableDict.Count; i++)
 {
 var kvp = tableDict.ElementAt(i);
 var s = kvp.Key;
 // Key is "( something )" - entry is constructor
 if (brackets.IsMatch(s))
 {
 var constructor = s.WhitespaceRemoved();
 constructor = constructor.Substring(1, constructor.Length - 2); // Remove brackets
 var argsAsStrings = constructor.Split(',');
 var args = argsAsStrings.Select(num => (object)int.Parse(num)).ToArray();
 // I have only implemented this for the Color class.
 // I assume the constructor is comprised of ints.
 list.Add(new RawChanceTableEntry<T>(
 (T)Activator.CreateInstance(typeof(T), args),
 Convert.ToDouble(kvp.Value)));
 }
 else
 {
 // Parse string as a property of T instead
 var type = typeof(T);
 list.Add(new RawChanceTableEntry<T>(
 (T)type.GetProperty(s).GetValue(Activator.CreateInstance(type), null),
 Convert.ToDouble(kvp.Value)));
 }
 }

I also create a specific RegEx for the bracket matching that should be faster as it's compiled at the class level:

 private static readonly Regex brackets = new Regex(@"\(([^\)]+)\)", RegexOptions.Compiled);

• jessehouwing: It's probably a lot faster to just check the first and last character using string.Equals than using a regex...
• Jesse C. Slicer: Probably? Have you measured?
• Jesse C. Slicer: I've just whipped up a program to measure this, and the regex wins out over 100,000 iterations for the string "(192, 168, 0)" - 75% faster. If I remove the closing parenthesis, the regex is 50% slower. If I remove the opening parenthesis, the regex is 75% faster again. Removing both parenthesis, the regex is still 75% faster. So, I think, over the long term, regex will perform pretty well. Looks like (on my computer) that ~2500 iterations is the tipping point where it performs better.
• jessehouwing: You seem to be correct. Turns out that the following is even faster: var trimmed = text.Trim(); if (trimmed[0] == '(' && trimmed[text.Length-1] == ')')

Answer 2

1. As you don't actually need the index of your tableDict, the more idiomatic way would be to use a foreach loop.
2. From your earlier review requests I do remember that the properties are static properties on the Color class in which case you do not need to create an instance in order to obtain the value for the property.
3. When using a regular expression you should make use of the grouping which will remove the need to split it up afterwards. Alternatively use StartsWith and EndsWith which would probably be faster than regex matching.
   • With grouping the regex could look like this: \s*\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)\s* (the \s* is to match any whitespaces), then you can use the Group property of the Match to get the 3 numbers directly.
4. You have code duplication in both code paths for parsing the probability and adding the new entry to the list.

So the code could look like this (based on StartsWith and EndsWith):

foreach (var kvp in tableDict)
{
 string s = kvp.Key.Trim(); // remove leading and trailing white spaces
 T item;
 if (s.StartsWith("(") && s.EndsWith(")")) 
 // Key is "( something )" - entry is constructor
 {
 var sCon = s.Substring(1, s.Length - 2); // Remove brackets 
 var args = sCon.Split(',')
 .Select(num => (object)int.Parse(num))
 .ToArray();
 // I have only implemented this for the Color class.
 // I assume the constructor is comprised of ints.
 T item = (T)Activator.CreateInstance(typeof(T), args);
 }
 else // Parse string as a static property of T instead
 {
 T item = typeof(T).GetProperty(s).GetValue(null);
 }
 double prob = Convert.ToDouble(kvp.Value);
 var entry = new RawChanceTableEntry<T>(item, prob);
 list.Add(entry);
}