Refactoring class to get rid of switch case

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

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.


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.

Tags:

C#

Recent Posts