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 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: https://kemiller2002.github.io/2016/03/07/Configuring-Logic.html.

Answer 2

Define a look up table array 3 by 2. Look up rate value in array cell adjacent to transport type. Calculate cost based on rate.

Answer 3

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 4

I prefer to use Enum for that like this:

public enum TransportMethod
{
    Bicycle = 1,
    Bus = 2,
    Car = 3
}

And use it like this method:

public decimal CostOfTravel(string transportMethod)
{
    var tmValue = (int)Enum.Parse(typeof(TransportMethod), transportMethod);
    return DistanceToDestination  * tmValue;
}

Note that above method is case-sensitive, So you can capitalize first char;

^{Related Answer}

Answer 5

Sounds like a good candidate for dependency-injection:

interface ITransportation {
    decimal CalcCosts(double distance);
}

class Bus : ITransportation { 
    decimal CalcCosts(double distance) { return (decimal)(distance * 2); }
}
class Bicycle : ITransportation { 
    decimal CalcCosts(double distance) { return (decimal)(distance * 1); }
}
class Car: ITransportation {
    decimal CalcCosts(double distance) { return (decimal)(distance * 3); }
}

Now you can easily create a new class Plane:

class Plane : ITransportation {
    decimal CalcCosts(double distance) { return (decimal)(distance * 4); }
}

Now create a constrcutor for your calculator that expects an instance of ITransportation. Within your CostOfTravel-method you can now call ITransportation.CalcCosts(DistanceToDestination).

var calculator = new TransportationCostCalculator(new Plane());

This has the advantage that you can exchange your actual transportation-instance without any code-change to your TransportationCostCalculator-class.

To complete this design you might also create a TransportationFactory as follows:

class TransportationFactory {
    ITransportation Create(string type) {
        switch case "Bus": return new Bus(); break
        // ...
}

Which you call like

ITransportation t = myFactory.Create("Bus");
TransportationCostCalculator calculator = new TransportationCostCalculator(t);
var result = myCalculator.CostOfTravel(50);

Answer 6

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 7

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);       
    }

Answer 8

This is a case for the strategy design pattern. Create a base class, say TravelCostCalculator, then develop classes for each mode of travel you will consider, each overriding a common method, Calculate(double). You can then instantiate the specific TravelCostCalculator as needed using the factory pattern.

The trick is in how to construct the factory (without a switch statement). The way I do this is by having a static class constructor (public static Classname() - not an instance constructor) that registers each strategy class with the factory in a Dictionary<string, Type>.

Since C# does not run class constructors deterministically (like C++ does in most cases) you have to explicitly run them to ensure they will run. This could be done in the main program or in the factory constructor. The downside is that if you add a strategy class, you must also add it to the list of constructors to be run. You can either create a static method that must be run (Touch or Register) or you can also use System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor.

class Derived : Base
{
    public static Derived()
    {
        Factory.Register(typeof(Derived));
    }
}

// this could also be done with generics rather than Type class
class Factory
{
    public static Register(Type t)
    {
        RegisteredTypes[t.Name] = t;
    }
    protected Dictionary<string, Type t> RegisteredTypes;

    public static Base Instantiate(string typeName)
    {
        if (!RegisteredTypes.ContainsKey(typeName))
            return null;
        return (Base) Activator.CreateInstance(RegisteredTypes[typeName]);
    }
}

Answer 9

You could define an abstract class like this, and have each TransportationMethod extend the abstract class:

abstract class TransportationMethod {
    public TransportationMethod() {
        // constructor logic
    }

    abstract public double travelCost(double distance);
}

class Bicycle : TransportationMethod {
    public Bicycle() : base() { }

    override public double travelCost(double distance) {
        return distance * 1;
    }
}

class Bus : TransportationMethod {
    public Bus() : base() { }

    override public double travelCost(double distance) {
        return distance * 2;
    }
}

class Car : TransportationMethod {
    public Car() : base() { }

    override public double travelCost(double distance) {
        return distance * 3;
    }
}

So in your actual method call, it could be rewritten like this:

public decimal CostOfTravel(TransportationMethod t) {
    return t.travelCost(DistanceToDestination);
}

Answer 10

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 11

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;
    }
}
....