Or you could look it up

I look at a lot of code. Code that was written a long time ago and those coders have long since left the building. Code that someone once cut their C# teeth on. Code that only its mother could love.

A common shortcoming is writing out what should be a table look up with flow control logic. Here’s an example of what I mean.

enum AccountType { Retail, Wholesale, Nonprofit }

private static decimal GetDiscountUsingIf( AccountType type )
{
  if ( type == AccountType.Wholesale )
    return 0.2M;
  else if ( type == AccountType.Nonprofit )
    return 0.1M;
  else if ( type == AccountType.Retail )
    return 0.0M;

  return 0.0M;
}

There’s no joy in this code. If the enumeration is prone to change over time, support and maintenance of this style of code quickly becomes error prone. And why is there a default return value at the end of the GetDiscountUsingIf method? Is it a hedge against additional values being added to the enumerations?

Only slightly better would be to convert if/else-if structure to a switch statement.

private static decimal GetDiscountUsingSwitch( AccountType type )
{
  switch ( type )
  {
    case AccountType.Wholesale:
      return 0.2M;
    case AccountType.Nonprofit:
      return 0.1M;
    case AccountType.Retail:
      return 0.0M;
    default:
      return 0.0M;
  }
}

This time the reason for having a default clause is that without it the compiler will warn that not all code paths return a value. Switch statements are cumbersome (e.g., with the potential for fall-throughs, etc.), the performance isn’t great (average number of comparisons is N/2, and worst case in N comparisons), and I think switch statements in general tend to be overused. So every time you see a switch statement I encourage you to think hard if it can be replaced. In this case, it is easily replaced with a Dictionary from the System.Collections.Generic namespace.

using System.Collections.Generic;

private static readonly Dictionary<AccountType, decimal> discounts =
  new Dictionary<AccountType, decimal>
  {
    { AccountType.Wholesale, 0.2M },
    { AccountType.Nonprofit, 0.1M },
    { AccountType.Retail, 0.0M }
  };

private static decimal GetDiscountUsingDictionary( AccountType type )
{
  if ( discounts.ContainsKey( type ) )
    return discounts[ type ];
  throw new ArgumentOutOfRangeException( "type" );
}

The discounts dictionary structure separates the relationship of the keys and values from the lookup logic. The indexer property, discounts[ type ], does most of the lookup logic, and at O(1) it is more efficient than the O(N) linear search approach of the previous two solutions. With the key-value association and lookup work taken care of with pretty terse syntax, the code is simple enough that I can contemplate what should be done with an AccountType key that’s not in the table, in this case throwing an exception that the argument is out of range.

So the next time you’re got a job that is essentially a dictionary look up, use a Dictionary to look it up.

Comments are closed.