.Net 4.0 Optimized code for refactoring existing "if" conditions and "is" operator

I have following C# code. It works fine; but the GetDestination() method is cluttered with multiple if conditions by using is operator.

In .Net 4.0 (or greater) what is the best way to avoid these “if” conditions?

EDIT: Role is part of the business model, and the destination is purely an artifact of one particular application using that business model.

CODE

public class Role { }
public class Manager : Role { }
public class Accountant : Role { }
public class Attender : Role { }
public class Cleaner : Role { }
public class Security : Role { }

class Program
{
    static string GetDestination(Role x)
    {
        string destination = @"\Home";

        if (x is Manager)
        {
            destination = @"\ManagerHomeA";
        }

        if (x is Accountant)
        {
            destination = @"\AccountantHomeC";
        }

        if (x is Cleaner)
        {
            destination = @"\Cleaner";
        }

        return destination;

    }

    static void Main(string[] args)
    {
        string destination = GetDestination(new Accountant());
        Console.WriteLine(destination);
        Console.ReadLine();
    }
}

REFERENCES

  1. Dictionary<T,Delegate> with Delegates of different types: Cleaner, non string method names?
  2. Jon Skeet: Making reflection fly and exploring delegates
  3. if-else vs. switch vs. Dictionary of delegates
  4. Dictionary with delegate or switch?
  5. Expression and delegate in c#
Jon Skeet
people
quotationmark

Here's one option:

private static readonly Dictionary<Type, string> DestinationsByType =
    new Dictionary<Type, string> 
{
    { typeof(Manager), @"\ManagerHome" },
    { typeof(Accountant), @"\AccountantHome" },
    // etc
};

private static string GetDestination(Role x)
{
    string destination;
    return DestinationsByType.TryGetValue(x.GetType(), out destination)
        ? destination : @"\Home";
}

Note:

  • This doesn't cope with null parameters. It's not clear whether or not you actually need it to. You can easily add null handling though.
  • This doesn't copy with inheritance (e.g. class Foo : Manager); you could do that by going up the inheritance hierarchy if necessary

Here's a version which does deal with both of those points, at the cost of complexity:

private static string GetDestination(Role x)
{
    Type type = x == null ? null : x.GetType();
    while (type != null)
    {
        string destination;
        if (DestinationsByType.TryGetValue(x.GetType(), out destination))
        {
            return destination;
        }
        type = type.BaseType;
    }
    return @"\Home";
}

EDIT: It would be cleaner if Role itself had a Destination property. This could either be virtual, or provided by the Rolebase class.

However, it could be that the destination is really not something the Role should concern itself with - it could be that Role is part of the business model, and the destination is purely an artifact of one particular application using that business model. In that sort of situation, you shouldn't put it into Role, as that breaks separation of concerns.

Basically, we can't tell which solution is going to be most suitable without knowing more context - as is so often the way in matters of design.

people

See more on this question at Stackoverflow