Refactoring class to get rid of switch case

Question

Say I have a class like this for calculating the cost of travelling different distances with different modes of transportation:

public class TransportationCostCalculator
{
    public double DistanceToDestination { get; set; }

    public decimal CostOfTravel(string transportMethod)
    {
        switch (transportMethod)
        {
            case "Bicycle":
                return (decimal)(DistanceToDestination * 1);
            case "Bus":
                return (decimal)(DistanceToDestination * 2);
            case "Car":
                return (decimal)(DistanceToDestination * 3);
            default:
                throw new ArgumentOutOfRangeException();
        }
    }

This is fine and all, but switch cases can be a nightmare to maintenance wise, and what if I want to use airplane or train later on? Then I have to change the above class. What alternative to a switch case could I use here and any hints to how?

I'm imagining using it in a console application like this which would be run from the command-line with arguments for what kind of transportation vehicle you want to use, and the distance you want to travel:

class Program
{
    static void Main(string[] args)
    {
        if(args.Length < 2)
        {
            Console.WriteLine("Not enough arguments to run this program");
            Console.ReadLine();
        }
        else
        {
            var transportMethod = args[0];
            var distance = args[1];
            var calculator = new TransportCostCalculator { DistanceToDestination = double.Parse(distance) };
            var result = calculator.CostOfTravel(transportMethod);
            Console.WriteLine(result);
            Console.ReadLine();
        }
    }
}

Any hints greatly appreciated!

Answer 1

You could use a strategy class for each type of travel. But, then you'd probably need a factory to create the strategy based upon the transport method which would likely have a switch statement to return the appropriate calculator.

    public class CalculatorFactory {
        public static ICalculator CreateCalculator(string transportType) {
            switch (transportType) {
                case "car":
                    return new CarCalculator();
                ...
public class CarCalculator : ICalculator {
    public decimal Calc(double distance) {
        return distance * 1;
    }
}
....

Answer 2

I think the answer is some kind of database.

If you use some, the TransportCostCalculator ask the database for the multiplayer to the given transportmethod.

The database may be a text-file or an xml or an SQL-server. Simply a key-value-pair.

If you want to use code-only there is - tmo - no way to avoid the translation from transportmethod to multiplayer (or cost). So some kind of swicht is needed.

With the database you put the dictionary out of your code and you must not change your code to apply new transportmethods or change the values.

Answer 3

You could do something like this:

public class TransportationCostCalculator {
    Dictionary<string,double> _travelModifier;

    TransportationCostCalculator()
    {
        _travelModifier = new Dictionary<string,double> ();

        _travelModifier.Add("bicycle", 1);
        _travelModifier.Add("bus", 2);
        _travelModifier.Add("car", 3);
    }


    public decimal CostOfTravel(string transportationMethod) =>
       (decimal) _travelModifier[transportationMethod] * DistanceToDestination;
}

You could then load the transportation type and it's modifier in a configuration file instead of using a switch statement. I put it in the constructor to show the example, but it could be loaded from anywhere. I would also probably make the Dictionary static and only load it once. There is no need to keep populating it each time you create a new TransportationCostCalculator especially if it isn't going to change during runtime.

As noted above, here is how you could load it by a configuration file:

void Main()
{
  // By Hard coding. 
  /*
    TransportationCostCalculator.AddTravelModifier("bicycle", 1);
    TransportationCostCalculator.AddTravelModifier("bus", 2);
    TransportationCostCalculator.AddTravelModifier("car", 3);
  */
    //By File 
    //assuming file is: name,value
    System.IO.File.ReadAllLines("C:\\temp\\modifiers.txt")
    .ToList().ForEach(line =>
        {
           var parts = line.Split(',');
        TransportationCostCalculator.AddTravelModifier
            (parts[0], Double.Parse(parts[1]));
        }
    );

}

public class TransportationCostCalculator {
    static Dictionary<string,double> _travelModifier = 
         new Dictionary<string,double> ();

    public static void AddTravelModifier(string name, double modifier)
    {
        if (_travelModifier.ContainsKey(name))
        {
            throw new Exception($"{name} already exists in dictionary.");
        }

        _travelModifier.Add(name, modifier);
    }

    public double DistanceToDestination { get; set; }

    TransportationCostCalculator()
    {
        _travelModifier = new Dictionary<string,double> ();
    }


    public decimal CostOfTravel(string transportationMethod) =>
       (decimal)( _travelModifier[transportationMethod] * DistanceToDestination);
}

Edit: It was mentioned in the comments that this wouldn't allow the equation to be modified if it ever needed to change without updating the code, so I wrote up a post about how to do it here: http://structuredsight.com/2016/03/07/configuring-logic.

Answer 4

It looks to me like any solution based on your current method is flawed in one critical way: No matter how you slice it, you're putting data in your code. This means every time you want to change any of these numbers, add a new vehicle type, etc., you have to edit code, and then recompile, distribute a patch, etc.

What you really should be doing is putting that data where it belongs - in a separate, non-compiled file. You can use XML, JSON, some form of database, or even just a simple config file. Encrypt it if you want, not necessarily needed.

Then you'd simply write a parser that reads the file and creates a map of vehicle type to cost multiplier or whatever other properties you want to save. Adding a new vehicle would be as simple as updating your data file. No need edit code or recompile, etc. Much more robust and easier to maintain if you plan to add stuff in the future.

Answer 5

It was said before but i want to give related topic another shot.

This is a good example for reflection. "Reflection objects are used for obtaining type information at runtime. The classes that give access to the metadata of a running program are in the System.Reflection namespace."

By using reflection, you will avoid compiling code if another switch type such as train is wanted to add the program. You will solve the problem on the fly by using a config file.

I recently solved a similar problem with strategy patttern, by using dependency injection but I still end up with switch statement. It doesnt solve your problem this way. Method suggested by tyson still needs recompile if a new type added to dictionary.

An example of what i am talking about: Dynamic Loading of Custom Configuration XML using Reflection in C# : http://technico.qnownow.com/dynamic-loading-of-custom-configuration-xml-using-reflection-in-c/

Answer 6

You can make a Dictionary that returns a multiplier based on transport.

public class TransportationCostCalculator
{
    Dictionary<string, int> multiplierDictionary;

    TransportationCostCalculator () 
    {
         var multiplierDictionary= new Dictionary<string, int> (); 
         dictionary.Add ("Bicycle", 1);
         dictionary.Add ("Bus", 2);
         ....
    }

    public decimal CostOfTravel(string transportMethod)
    {
         return  (decimal) (multiplierDictionary[transportMethod] * DistanceToDestination);       
    }