Constructor Injection, new dependency and its impact on code

Question

This is my interface, all of my transport concrete classes will be implementing this interface.

interface ITransport
{
    void Travel()
}

These are my existing implementation class:

class Horse : ITransport
{
    public Horse()
    {
    }

    public void Travel()
    {
        Console.Writeline("I am travelling on a horse");
    }
}

class Camel : ITransport
{ 
    public Camel()
    {
    }

    public void Travel()
    {
        Console.Writeline("I am travelling on a Camel");
    }
}

class Ship : ITransport
{
    public Ship()
    {
    }

    public void Travel()
    {
        Console.Writeline("I am travelling on a Ship");
    }
}

This is my Factory class I create instance of my transport classes from this factory class.

class TransportFactory
{
    public TransportFactory()
    {
    }

    ITransport ProvideTransport(string transportType)
    {
        switch(transportType)
        {
            case "Camel": return Camel; break;
            case "Horse": return Horse; break;
            case "Ship": return Ship; break;
        }
    }
}

This is my creational class meaning I configure my client classes or consumer classes through this class:

class ConfigurationBuilder
{
    ITransport camel;
    ITransport ship;
    ITransport horse;

    public ConfigurationBuilder()
    {
        TransportFactory tFactory = new TransportFactory();
        horse = tFactory.ProvideTransport("horse");
        camel = tFactory.ProvideTransport("camel");
        ship = tFactory.ProvideTransport("ship");
    }

    Human ConfigureHuman()
    {
        return new Human(camel, ship, horse);
    }
}

This is my client code the actual caller of these transport class:

class Human
{
    ITransport camel;
    ITransport ship;
    ITransport horse;

    public Human(ITransport camel, ITransport ship, ITransport horse)
    {
        this.camel = camel;
        this.ship = ship;
        this.horse = horse; 
    } 

    void Travel()
    { 
        //TransportFactory tFactory = new TransportFactory(); 

        if(ground =="plain")
        { 
            horse.Travel(); 
        } 
        if(ground =="desert")
        {
            camel.Travel(); 
        } 
        if(ground =="sea")
        { 
            ship.Travel(); 
        } 
    }
}

Now my question is What will happen when I have a new means of traveling: such as Plane for Air and then Rocket for Space etc.

My Constructor will change and will keep on changing as more and more means of traveling keep on adding throughout application life time. Is this the correct design or is there a better way.

As for the question is considered I couldn't come up with intuitive words to explain my problem :)

Update: Commented TransportFactory out of Human class

Update2:

I guess I now understood how it will be done. I will be needing two factories now.TransportFactory which is already implemented above, the second one will be HumanFactory which will create a human but also has additional responsibility of calling travel method of appropriate ITransport.

public class HumanFactory
{
    ITransport camel;
    ITransport ship;
    ITransport horse;
    Human _human;
    Dictionary<string, ITransport> _availableTransports;
    event Action<Human, string> transportRequested;

    public HumanFactory(TransportFactory tFactory)
    {

        horse = tFactory.ProvideTransport(TransportTypes.Horse);
        camel = tFactory.ProvideTransport(TransportTypes.Camel);
        ship = tFactory.ProvideTransport(TransportTypes.Ship);
    }

    public Human ConfigureHuman()
    {
        if (_availableTransports == null)
        {
            _availableTransports = new Dictionary<string, ITransport>();
            _availableTransports.Add(GroundTypes.Desert.ToString(), camel);
            _availableTransports.Add(GroundTypes.Sea.ToString(), ship);
            _availableTransports.Add(GroundTypes.Plains.ToString(), horse);
        }

        transportRequested += new Action<Human, string>(_human_transportRequested);
        _human = new Human(transportRequested);

        return _human;
    }

    void _human_transportRequested(Human human, string groundType)
    {
        if (_availableTransports.ContainsKey(groundType))
        {
            ITransport suitableTransport = _availableTransports[groundType];
            suitableTransport.Travel();
        }
        else
        {
            //code for handling below conditions goes here
            //I don't know what to do for this type of plain?
        }
    }
}

Here is the Human class now:

public class Human
{
    Action<Human, string> _transportRequested;

    public Human(Action<Human, string> transportRequested)
    {
        _transportRequested = transportRequested;
    }

    public void Travel()
    {
        if (_transportRequested != null)
        {
            var ev = _transportRequested;
            ev.Invoke(this, GroundTypes.Plains.ToString());
        }
    }
}

We will call the Human class now as:

TransportFactory tFactory=new TransportFactory();
HumanFactory humanFactory = new HumanFactory(tFactory);

Human human = humanFactory.ConfigureHuman();
human.Travel();

One More thing: I guess I should be having a lock in the HumanFactory class in _human_transportRequested method? In case there is a multithreaded scenario. I guess now my code is following Law of Demeter :)

Special Thanks to Nik( for that marvelous solution) and Gleb( for those wonderful videos)

Answer 1

I assume that this is just an example and I don't need to tell you that you should use enums instead of strings, remove breaks after returns, indent your code, etc. :) So, to answer your question: I think you should refactor your Human class:

class Human
{
    private readonly TransportFactory _tFactory;

    public Human(TransportFactory tFactory)
    {
        if (tFactory == null) 
        {
            throw new ArgumentNullException("tFactory");
        }
        _tFactory = tFactory;
    }

    void Travel(string ground)
    { 
        var transport = _tFactory.ProvideTransport(ground);
        transport.Travel();
    }
}

class TransportFactory
{
    ITransport ProvideTransport(string groundType)
    {
        switch(groundType)
        {
            case "desert":
                return new Camel();
            case "plain":
                return new Horse();
            case "sea":
                return new Ship();
            default:
                return new ArmsAndLegs();
        }
    }
}

You will still need to modify factory method when you add new transport though. This can be avoided by using reflection.

Edit: here is another example:

class Human
{
    public event Action<Human, ПкщгтвЕнзуы> TransportRequested;

    void Travel(GroundTypes ground)
    { 
        var ev = TransportRequested;
        if (ev != null)
        {
            ev(this, ground);
        }
    }
}

interface ITransport
{
    void Transport(object cargo);
}


class TravelAgency
{
    private readonly TransportFactory _tFactory;

    public TravelAgency(TransportFactory tFactory)
    {
        if (tFactory == null) 
        {
            throw new ArgumentNullException("tFactory");
        }
        _tFactory = tFactory;
    }

    //this method at some point subscribes to TransportRequested event
    private void OnTransportRequested(Human human, GroundTypes ground)
    {
        var transport = _tFactory.ProvideTransport(ground);
        transport.Transport(human);
    }
}

Edit2

Subscription using your TransportFactory logic:

public class HumanFactory
{
     Dictionary<GroundTypes, ITransport> _availableTransports;

     public HumanFactory(TransportFactory tFactory)
     {
        _availableTransports = new Dictionary<string, ITransport>();
        _availableTransports.Add(GroundTypes.Desert, tFactory.ProvideTransport(TransportTypes.Horse));
        _availableTransports.Add(GroundTypes.Sea, tFactory.ProvideTransport(TransportTypes.Camel));
        _availableTransports.Add(GroundTypes.Plains, tFactory.ProvideTransport(TransportTypes.Ship));
     }

     public Human CreateHuman()
     {
         var h = new Human();
         h.TransportRequested += OnHumanTransportRequested;
         return h;
     }

     //you can incapsulate this logic in some other object
     //you should also consider implementing Events Aggregator
     //or those subscriptions might become quite complicated as their number increases
     void OnHumanTransportRequested(Human human, GroundTypes groundType)
     {
         if (_availableTransports.ContainsKey(groundType))
         {
             ITransport suitableTransport = _availableTransports[groundType];
             suitableTransport.Travel();
         }
         else
         {
             //code for handling below conditions goes here
             //I don't know what to do for this type of plain?
         }
     }
 }

Answer 2

Sticking to your example, I came up with the following:

1) Interface for Transports

interface Transport
{
    void Travel();
    bool isSuitableForGround(Ground ground);
}

2) Possible implementation

class Horse:Transport
{
    HashSet<Ground> suitableGrounds = new HashSet<Ground> { Ground.Plain };
    public bool isSuitableForGround(Ground ground)
    {       
            return suitableGrounds.Contains(ground);
    }

    public void Travel()
    {

    }
}

So. The Horse knows best, upon which grounds it would walk.

3) The Factory should know nothing of the abilities of what it delivers except the type. The ground which a transport is suitable for is up to the transport itself. It knows best.

class TransportationsFactory
{
    Dictionary<Type, TransportFactory> transporters=new Dictionary<Type,TransportFactory>();

    public Transport GetTransportType<T>(){
        if (transporters.ContainsKey(typeof(T)) == false) throw new Exception();
        return transporters[typeof(T)].produce();
    }

    public TransportationsFactory(){
        transporters.Add(typeof(Horse),new HorseBreeder());
    }

}

If you want a Horse, you get a Horse by

transportationsFactory.GetTransportType<Horse>()

4) The implementation of a human looks like the following:

class Human
{
    List<Transport> transports = new List<Transport>();

    public void TravelOn(Ground ground){
        try
        {
            Transport possibleTransports = transports.Where(t => t.isSuitableForGround(ground)).First();
            possibleTransports.Travel();
        }
        catch (InvalidOperationException ex) { 
            //Do something useful
        }

    }

    public Human(IEnumerable<Transport> transports){
        this.transports = transports.ToList();
    }
}

Of course you could choose another strategy to get a suitable transport. For the sake of the example I chose the strategy to get the first one able to do the job.

Since you asked for constructor injection, I implemented it this way, taking a collection of transports. I would prefer setter injection.

If there is the need to travel on a (different) ground, the human asks its collection of transports, whether one of it is capable of doing the job - so the human too has not to have any knowledge at all what is good for what job.

If you ever come up with a car, you add it to your factory and ask your factory for that. And the car knows best, for what grounds it is made for.

Answer 3

At first there are a lot of random "new" operators. It is best to compose application on it entry point like:

 public class Program 
 {
         public static void Main()
         {
            IDictionary<GroundType, ITransport> transports = new Dictionary<GroundType, ITransport> 
            {
                { GroundType.Plain, new Horse() },
                { GroundType.Desert, new Camel() },
                { GroundType.Sea, new Ship() }
            };


            TransportFactory transportFactory = new TransportFactory(transports);

            Human hero = new Human(transportFactory);
            hero.Travel();
        }
}

    public class Human
    {
        private readonly TransportFactory _transportFactory;

        private GroundType _ground;

        public Human(TransportFactory transportFactory)
        {
            _transportFactory = transportFactory;
        }

        public void Travel()
        {
            var transport = _transportFactory.GetTransport(_ground);
            transport.Travel();
        }
    }

    public class TransportFactory
    {
        private readonly IDictionary<GroundType, ITransport> _transports;

        public TransportFactory(IDictionary<GroundType, ITransport> transports)
        {
            _transports = transports;
        }

        // virtual will allow to override this method with test stub\mock
        public virtual ITransport GetTransport(GroundType groundType)
        {
            ITransport transport;
            if (!_transports.TryGetValue(groundType, out transport)) {
                // this way we can find any issues earlier 
                throw new NotSupportedException("The current ground of type '{0}' is not supported");
            }

            return transport;
        }
    }

The full DI Composition Root might look after some time:

public static void Main()
{
    IDictionary<GroundType, ITransport> transports = new Dictionary<GroundType, ITransport> 
    {
        { GroundType.Plain, new Horse() },
        { GroundType.Desert, new Camel() },
        { GroundType.Sea, new Ship() }
    };


    TransportFactory transportFactory = new TransportFactory(transports);

    Human hero = CreateHuman(transportFactory);
    Human enemy = CreateHuman(transportFactory);

    Continent continent = Continent.LoadFromResource(DefaultContinent);
    continent.Place(hero, new Vector3(0, 0, 0));
    continent.Place(enemy, new Vector3(110, 30, 0));

    Continent demonicContinent = Continent.LoadFromResource(DemonicContinent);

    World world = new World(new List<Continent> { continent, demonicContinent });
    Game game = new Game(world, new DefaultRulesProvider());
    game.Start();
}

I also suggest to read Mark Seemann's "Dependency Injection in .NET" book.

There are also a lot of talks about testability, DI, and OOP by Misco Hevery - I would have watch them 4 years earlier!