11

I've got a situation where I have a business object with about 15 properties of different types. The business object also has to implement an interface which has the following method:

object GetFieldValue(string FieldName);

I can see 2 ways of implementing this method:

Use a switch statement:

switch ( FieldName )
{
    case "Field1": return this.Field1;
    case "Field2": return this.Field2;
    // etc.
}

Use a dictionary (SortedDictionary or HashTable?):

return this.AllFields[FieldName];

Which would be more efficient?

Added: Forgot to say. This method is for displaying the item in a grid. The grid will have a column for each of these properties. There will routinely be grids with a bit over 1000 items in them. That's why I'm concerned about performance.

Added 2:

Here's an idea: a hybrid approach. Make a static dictionary with keys being property names and values being indices in array. The dictionary is filled only once, at the startup of the application. Every object instance has an array. So, the lookup would be like:

return this.ValueArray[StaticDictionary[FieldName]];

The dictionary filling algorithm can use reflection. The properties itself will then be implemented accordingly:

public bool Field1
{
    get
    {
        object o = this.ValueArray[StaticDictionary["Field1"]]; 
        return o == null ? false : (bool)o;
    }
    set
    {
        this.ValueArray[StaticDictionary["Field1"]] = value;
    }
}

Can anyone see any problems with this?

It can also be taken one step further and the ValueArray/StaticDictionary can be placed in a separate generic type ValueCollection<T>, where T would specify the type for reflection. ValueCollection will also handle the case when no value has been set yet. Properties could then be written simply as:

public bool Field1
{
    get
    {
        return (bool)this.Values["Field1"];
    }
    set
    {
        this.Values["Field1"] = value;
    }
}

And in the end, I'm starting to wonder again, if a simple switch statement might not be both faster and easier to maintain....

Vilx-
  • 104,512
  • 87
  • 279
  • 422
  • Is there a reason you're not binding the whole object to the grid as a datarow? – Rowland Shaw Aug 26 '09 at 12:58
  • To tell the truth, it's the DevExpress TreeList thingy. It's like a treeview/gridview hybrid. So the data has to be hierarchial. And the interface is there so that the TreeList understands the hierarchy 'n stuff. I could probably also translate it all to a DataTable (it can bind to that too), but this is more comfortable for me to work with afterwards. – Vilx- Aug 26 '09 at 14:31
  • I mean, I will do other stuff with this data structure later too, not just display it in the grid. – Vilx- Aug 26 '09 at 14:32
  • why not using reflection? [see this](http://msdn.microsoft.com/en-us/library/b05d59ty.aspx) [A blog about reflection and performance](http://www.west-wind.com/weblog/posts/351.aspx) [Another good article about reflection](http://msdn.microsoft.com/en-ca/magazine/cc163759.aspx) – Fredou Aug 26 '09 at 11:36
  • 1
    Shouldn't that be slower than either of these? – Vilx- Aug 26 '09 at 11:37
  • is performance, in millisecond, important? – Fredou Aug 26 '09 at 11:48
  • The object will be displayed in a grid which will query the properties. I'll routinely have about 1000 items in such a grid. That's 15'000 reflection calls. I shudder to think what would happen if each of them took 1ms. – Vilx- Aug 26 '09 at 12:02
  • ho, ok that is pretty big maybe reflection is not a good option in your case – Fredou Aug 26 '09 at 12:41

4 Answers4

22
switch:      good efficiency, least maintainable
dictionary:  good efficiency, better maintainability
reflection:  least efficient, best maintainability

Hint: ignore efficiency and worry about maintainability only, unless you've actually tested performance and found it be be an issue.

I'm not saying reflection is your only choice, just that it would allow you to add/remove and rename properties as needed, and not need to keep the switch statement or dictionary in sync.

Ash
  • 60,973
  • 31
  • 151
  • 169
  • Reflection would allow your properties to be discovered at run-time and so if you add/delete or rename any, you don't need to re-write any other code (such as the switch statement or dictionary initialisation). – Ash Aug 26 '09 at 11:44
  • 7
    I tend to find the cognitive overhead of reflection makes it somewhat less maintainable than the dispatch-table Dictionary approach. Though that may just be because I don't use reflection very often. :) – Greg D Aug 26 '09 at 11:50
  • 3
    A dictionary more efficient than a switch statement? I didn't expect that, could you elaborate? – Bubblewrap Aug 26 '09 at 11:52
  • @Bubblewrap. It could be for strings. For less sparse lookups a switch will be faster. – Mike Two Aug 26 '09 at 11:58
  • @Bubblewrap, I was almost going to write "equal efficiency" for switch and dictionary, but I'm just taking a general view that memory access at near O(1) is quicker then what the runtime can do with a switch lookup. This is exactly why I'm emphasizing the maintainability. I might change the wording anyway. – Ash Aug 26 '09 at 12:03
  • I modified the question a bit. Still think that reflection is the way to go? – Vilx- Aug 26 '09 at 12:05
  • @Greg, good point. Reflection isn't that hard, you just need to get familiar with the general "object tree" and how to navigate around it. What I do though is encapsulate reflection code into helper attribute classes that you can then utilise by simply adding atributes to your code. – Ash Aug 26 '09 at 12:06
  • @Vilx, as I'm comfortable with Reflection, I'd simply compare it with the dictionary or switch (using the Stopwatch class) and then decide. Yes, 15,000 calls to a version of GetFieldValue() that uses reflection may be a performance issue, but you need to confirm it is. – Ash Aug 26 '09 at 12:09
  • @Vilx, many domain/business object frameworks such as CSLA.NET allow you to "turn on/off" Reflection to enumerate object properties for exacylt the situation you describe (showing a grid). – Ash Aug 26 '09 at 12:15
  • OK, I'll profile it. It's true - for maintenance reflection is the easiest way (I'm comfortable with it too). But I really think it might easily eat up a couple of seconds while the grid opens up. Something I'd like to avoid. – Vilx- Aug 26 '09 at 12:22
  • @Vilx, your hybrid approach would work in theory, but you're absolutely right about the extra maintainability issues it could introduce. I'd probably go with the simple dictionary or switch approach. The hybrid approach would be starting to "reinvent the wheel" in that other frameworks (CSLA) do tat sort of thing for you. – Ash Aug 26 '09 at 12:55
  • My application is pretty lightweight and I don't want to introduce ORM frameworks or the like. Especially because the data I'm visualing does not come in a neat tabular format from a RDBMS. However, the hybrid dictionary above should be superior to a simple dictionary in every aspect, including maintainability. The switch statement would be simpler though. – Vilx- Aug 26 '09 at 13:00
  • The switch will usually be the faster one not the dictionary (with few entries that is) Actually based on the overhead of calculating the hashkey I wouldn't feal confident that reflection wasn't faster than the dictionary (without testing at least) – Rune FS Aug 26 '09 at 16:38
  • 2
    @Rune, I can guarantee you that a dictionary is quicker then using reflection. Hashtables & dictionarys in computer science exist because they can provide higher performance for retrieval of data then most other data structures. Whereas reflection exists to provide discoverability of types at run-time, not for performance reasons. But don't take my word for it, try a comparison as an experiment. – Ash Aug 27 '09 at 03:21
7

Because you are using strings Dictionary will probably be faster. Switch will essentially get translated to a hashtable when using strings. But if you are using ints or similar it gets translated to a jump table and will be faster.

see this answer and question for more details

Best bet is to profile it and find for sure

Community
  • 1
  • 1
Mike Two
  • 44,935
  • 9
  • 80
  • 96
0

How you get the value of each property will probably have a lesser impact on the overall performance than how you render your grid.

Let me give you an example: let's say you have the following implementation:

private string _latestFieldName = string.Empty;
private PropertyInfo _propertyInfo;

object GetFieldValue(string FieldName)
{
  if(FieldName != _latestFieldName)
  {
    _propertyInfo = typeof(yourTypeName).GetProperty(FieldName);
  }
  return _propertyInfo.GetValue(this,null);
}

If your grid rendering is rendering a row at a time using reflection it will have to get the propertyinfo every single time. wheras if you render column by column you'd only have to get the propertyInfo once for each property and since the branch prediction will be right almost every time you'd only miss a very few clock cycles on the if. When you have the PropertyInfo already and you don't need to cast the result of the call to GetValue. using reflection comes VERY close to using the getter of a property when it comes to speed.

My point is before you start to optimize use a profiler. (Of course if you can't change the grid well you can't really optimize it either)

Rune FS
  • 21,497
  • 7
  • 62
  • 96
0

Switch actually has 2 benefits comparing to dictionary:

  1. You can create customized exception message in default section that can include invalid value. In case of dictionary you only get KeyNotFoundException that didn't contain key name.
  2. You can handle null values. Dictionary can't store null as a key.
dimaaan
  • 861
  • 13
  • 16