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

Question

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#

Answer 1

Approach 1 (Selected): Using dynamic keyword to implement multimethods / double dispatch

Approach 2: Use a dictionary to avoid if blocks as mentioned in Jon Skeet’s answer below.

Approach 3: Use a HashList with delegates if there is condition other than equality (For example, if input < 25). Refer how to refactor a set of <= , >= if...else statements into a dictionary or something like that

Apporach 4: Virtual Functions as mentioned in MarcinJuraszek’s answer below.

MultiMethods / Double Dispatch approach using dynamic keyword

Rationale: Here the algorithm changes based on the type. That is, if the input is Accountant, the function to be executed is different than for Manager.

    public static class DestinationHelper
    {
        public static string GetDestinationSepcificImplm(Manager x)
        {
            return @"\ManagerHome";
        }

        public static string GetDestinationSepcificImplm(Accountant x)
        {
            return @"\AccountantHome";
        }

        public static string GetDestinationSepcificImplm(Cleaner x)
        {
            return @"\CleanerHome";
        }
    }

   class Program
    {
        static string GetDestination(Role x)
        {

            #region Other Common Works
            //Do logging
            //Other Business Activities
            #endregion

            string destination = String.Empty;
            dynamic inputRole = x;
            destination = DestinationHelper.GetDestinationSepcificImplm(inputRole);
            return destination;
        }

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

    }

Answer 2

This is a strongly typed, imperative language so if statements and type checking are going to happen.

Having said that, have you considered a virtual method on Role that can be overridden to provide a destination string?

A further alternative, a lookup table!

Dictionary<Type, string> paths = new Dictionary<TYpe, string>()
{
    { typeof(Manager),  @"\ManagerHomeA" }
    { typeof(Accountant),  @"\AccountantHomeC" }
    { typeof(Cleaner),  "Cleaner" }
}

string path = @"\Home";
if(paths.ContainsKey(x.GetType())
    path = paths[x];

Answer 3

Role should have a virtual function that would return destination:

public virtual string GetDestination()
{
     return "Home";
}

And all the classes should override this function and return the correct string. Then in the code you would have:

var role = new Accountant();
string destination = role.GetDestination();

I hope that helps. There may be typos, I am writing from head.

Answer 4

Having virtual property which would be overriden in derived classes should do the trick:

class Role
{
    public virtual string Destination { get { return "Home"; } }
}
class Manager : Role
{
    public override string Destination { get { return "ManagerHome;"; } }
}
class Accountant : Role
{
    public override string Destination { get { return "AccountantHome;"; } }
}
class Attender : Role
{
    public override string Destination { get { return "AttenderHome;"; } }
}
class Cleaner : Role
{
    public override string Destination { get { return "CleanerHome;"; } }
}
class Security : Role { }

I didn't make the property abstract, to provide default Home value when it's not overriden in derived class.

Usage:

string destination = (new Accountant()).Destination;
Console.WriteLine(destination);
Console.ReadLine();

Answer 5

One way to do it would be to use a map instead of an if:

//(psuedocode)
private Dictionary<Type, string> RoleMap;

void SomeInitializationCodeThatRunsOnce()
{
  RoleMap.Add(typeof(Manager), @"\ManagerHome");
  RollMap.Add(typeof(Accountant), @"\AccountantHome");
  // ect...
}

string GetDestination(Role x)
{
  string destination;
  if(!RoleMap.TryGet(x.GetType(), out destination))
    destination = @"\Home";
  return destination;
}

Further reading: http://www.hanselman.com/blog/BackToBasicsMovingBeyondForIfAndSwitch.aspx

Answer 6

you can either use an interface definition or an abstract method / property

with interface:

public interface IDestinationProvider
{
    sting Destination { get; }
}

string GetDestination(Role role)
{
    var provider = role as IDestinationProvider;
    if (provider != null)
        return provider.Destination;
    return "Default";
}

with an abstract base class

abstract class Role 
{ 
    public abstract string GetDestination();
}

class Manager : Role
{
    public virtual string GetDestination() { return "ManagerHomeA"; }
}

string GetDestination(Role role)
{
    return @"\" + role.GetDestination();
}

or with attributes:

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
public class DestinationAttribute : Attribute
{
    public DestinationAttribute() { this.Path = @"\Home"; }
    public string Path { get; set; }
}

[Destination(Path = @"\ManagerHome")]
public class Manager : Role { }

string GetDestination(Role role)
{
    var destination = role.GetType().GetCustomAttributes(typeof(DestinationAttribute), true).FirstOrDefault();
    if (destination != null)
        return destination.Path;

    return @"\Home";
}

Answer 7

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.