1

I would like to know who (which object) creates the real object which gets dependeny injected.

To make it more understandable here my example code

IOrder:

interface IOrder
{
    int Id { get; set; }
    string Name { get; set; }
    bool SaveChanges();
}

Order:

public class Order : IOrder
{
    private IOrderRepository repository;

    public int Id { get; set; }
    public string Name { get; set; }

    public Order(IOrderRepository repository)
    {
        this.repository = repository;
    }

    public bool SaveChanges()
    {
        if (Id < 1)
            return repository.Save(this);
        else
            return repository.Update(this);
    }
}

Like we can see if i want to create an Orderobject i need to inject an some object which inherited from IOrderRepository

IOrderRepository:

interface IOrderRepository
{
    IList<IOrder> GetAll();
    IOrder GetById(int id);
    IList<IOrder> GetBySomeRefId(int SomeRefId);
    bool Save(IOrder order);
    bool Update(IOrder order);
}

OrderRepository:

public class OrderRepository : IOrderRepository
{
    public IList<IOrder> GetAll()        {        }

    public IOrder GetById(int id)        {        }

    public IList<IOrder> GetBySomeRefId(int SomeRefId)        {        }

    public bool Save(IOrder order)        {        }

    public bool Update(IOrder order)        {        }
}

So here each Get method can and will inject it one class as a dependency, but if I need a new Order I have to do

var myOrderRepos = new OrderRepository();
var myNewOrder = new Order(myorderrepos);

and here is my problem where ever I have to write the

var myOrderRepos = new OrderRepository();

this class will break the separation ...

And this will basically every ViewModel which has a new command in it in this case NewOrderCommand.

OrderListVM snipped

    public ICommand NewOrderCommand
    {
        get { return newOrderCommand ?? (newOrderCommand = new RelayCommand(param => this.OnNewOrder())); }
    }

    public void OnNewOrder()
    {
        var myOrderRepos = new OrderRepository(); // <= tight coupling to OrderRepository
        var myNewOrder = new Order(myorderrepos); // <= tight coupling to Order
        var myOrderVM = new OrderVM(myNewOrder);
        myOrderVM.Show();
    }

How would you avoid this tight coupling?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
WiiMaxx
  • 5,322
  • 8
  • 51
  • 89
  • My first thought was "it's turtles all the way down." – CoderDennis Feb 19 '15 at 08:30
  • 1
    Related: https://stackoverflow.com/questions/4835046/why-not-use-an-ioc-container-to-resolve-dependencies-for-entities-business-objec – Steven Feb 19 '15 at 09:39
  • 1
    What you are doing is applying the [Active Record Pattern](https://en.wikipedia.org/wiki/Active_record_pattern), which should be considered an anti-pattern, because it violates [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single_responsibility_principle) and couples your entities to the persistence. – Steven Feb 19 '15 at 09:41
  • @Steven mmh i see, so then i should inject my `IOrderRepository` into my `OrderVM` and use the `Save()` there right? – WiiMaxx Feb 19 '15 at 11:45
  • 1
    @WiiMaxx It depends on the complexity of your application. I would even say that saving is not a responsibility of your presentation layer, but of your business layer, and probably be some infrastructural (cross-cutting concern). So my general solution is to inject the repository into a [command handler](https://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=91) and inject the abstraction for that command handler into the ViewModel. – Steven Feb 19 '15 at 11:50

1 Answers1

2

Why not put a CreateOrder method in your OrderRepository? Then your view models would get an IOrderRepository injected into them and be able to create orders from there.

I wouldn't make Order depend on IOrderRepository. That seems like the wrong level to place the dependency. When you've got a repository that's doing the saving and other work, then keeping objects like Order as simple data containers and putting all the actions in the repository provides a clean separation.

I find when using dependency injection (especially when combined with repositories), my view model and controller code very rarely contains the new keyword.

Dependency Injection is a pattern for managing coupling. As marc_s points out in his comment, you want to depend on the abstractions, not the implementations. It's OK for your classes to have dependencies, but it's better if the interface for each dependency is given to your class than for your class to create its own version of the implementation of that dependency. That's why I don't see new in my classes. I know if I'm newing up an object then that's probably a good place to stop and identify how to pass that dependency into the class instead of creating it right there. Of course, a repository creating new instances of the data object it manages is a good place for a call to new.

Here's some code to try to help explain what I'm talking about. While working through this, I noticed you're creating OrderVM instances within your OrderListVM. So, I've taken the dependency on IOrderRepository out of the VMs and introduced a ViewModel factory. You may also want a RepositoryFactory, but that seemed like too much to show here.

IOrder:

interface IOrder
{
    int Id { get; set; }
    string Name { get; set; }
}

Order:

public class Order : IOrder
{
    public int Id { get; set; }
    public string Name { get; set; }
}

IOrderRepository:

interface IOrderRepository
{
    IList<IOrder> GetAll();
    IOrder GetById(int id);
    IList<IOrder> GetBySomeRefId(int SomeRefId);
    bool Save(IOrder order);
    bool Update(IOrder order);
    IOrder CreateNewOrder();
}

OrderRepository:

public class OrderRepository : IOrderRepository
{
    ...
    public IOrder CreateNewOrder()
    {
        return new Order();
    }
}

OrderListVM:

public class OrderListVM 
{

    ViewModelFactory _factory;
    IEnumerable<IOrder> _orderList;

    public OrderListVM(ViewModelFactory factory, IEnumerable<IOrder> orders)
    {
        _factory = factory;
        _orderList = orders;
    }

    public void OnNewOrder()
    {
        var myOrderVM = _factory.GetOrderVM();
        myOrderVM.Show();
    }
}

OrderVM

public class OrderVM 
{
    IOrder _order;
    public OrderVM(IOrder order)
    {
        _order = order;
    }
}

ViewModelFactory

interface IViewModelFactory { ... }

public class ViewModelFactory : IViewModelFactory 
{
    IOrderRepository _repository;

    public ViewModelFactory (IOrderRepository repo)
    {
        _repository = repo;
    }

    public OrderListVM GetOrderListVM() {
        return new OrderListVM(this, _repository.GetAll());
    }

    public OrderVM GetOrderVM(IOrder order = null) {
        if (order == null) {
            order = _repository.CreateNewOrder();
        }
        return new OrderVM(order);
    }

}

Somewhere at the root of your project you'll have some bootstrap code that creates an instance of the ViewModelFactory and other dependencies and injects them into the classes that need them.

CoderDennis
  • 13,642
  • 9
  • 69
  • 105
  • Thanks for your answer, but even if i do so my `OrderListVM`will be tight coupled to the OrderRepository which will even worse than a static `Order.CreateOrder`because it would couple my DAL to my view – WiiMaxx Feb 19 '15 at 08:38
  • 1
    The snipet you included in the question has `new OrderRepository()` inside a VM, so why not inject that dependency instead of hard coding it? – CoderDennis Feb 19 '15 at 08:42
  • to your edit, sure my it will be rare but if you client need to create a new order it has to be done somewhere in your code right? so the class contains this code will get tightly coupled to the other class – WiiMaxx Feb 19 '15 at 08:43
  • to your comment, sore i could also inject the `IOrderRepository` to my `OrderListVM` but some class will get tightly coupled to the `OrderRepository` and every other class and i would like to know how to separate this. – WiiMaxx Feb 19 '15 at 08:48
  • @WiiMaxx: you'll get a dependency on `IOrderRepository` (the **interface**) but **NOT** on `OrderRepository` (the **implementation**) - the caller could always inject another implementation of `IOrderRepository` - and that's **the whole point** - your code should rely on **abstractions** (interfaces) rather than actual implementations. – marc_s Feb 19 '15 at 08:50
  • @marc_s you are right, my whole problem is where do i set the `new` ? because it has to be done some where right? – WiiMaxx Feb 19 '15 at 09:04
  • @CoderDennis maybe i'm confused or you do not understand where my problem is :( , would it be possible for you to add some sample code to explain your thoughts/ answer? – WiiMaxx Feb 19 '15 at 09:07
  • @WiiMaxx: I think you need to keep two things apart: **dependencies** like the `IOrderRepository` class, should be **injected** as a dependency, and then you don't ever need to do a `new OrderRepository();` at all. **Data classes** like the `Order` class still need to be newed up at some place - like inside the `IOrderRepository.CreateNewOrder()` method that Dennis suggested – marc_s Feb 19 '15 at 09:12
  • @marc_s that's all true (even if i think the `CreateNewOrder()` should be somewhere else) , but like CoderDennis sayed "it's turtles all the way down.". The injection has to start some where with a real Object and i just want to know where i should do that to start the injection all the way down to the places where i need them – WiiMaxx Feb 19 '15 at 09:27
  • @CoderDennis ah i see very interesting, based on your code should all my `ViewModel` implemented in this Factory ? or should i create a separate one for each (e.g. `CostumerFactory` +`ICostumerFactory`) and if i should do that wouldn't i need a FactoryFactory to get the right instance `FactoryFactory` for each type? which is also an anti pattern. Now i'm confused :'( – WiiMaxx Feb 19 '15 at 13:13
  • @WiiMaxx I don't know if I can be much more help here. It's hard to know what to suggest for your application. I'm not even 100% sure that I like or would use the ViewModelFactory, but it was the best way to share what I was thinking. The comment about an abstract command handler sounds like a great idea. – CoderDennis Feb 19 '15 at 16:35