Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Say we have a task logging system, when a task is logged, the user specifies a category and the task defaults to a status of 'Outstanding'. Assume in this instance that Category and Status have to be implemented as entities. Normally I would do this:

Application Layer:

public class TaskService
{
    //...

    public void Add(Guid categoryId, string description)
    {
        var category = _categoryRepository.GetById(categoryId);
        var status = _statusRepository.GetById(Constants.Status.OutstandingId);
        var task = Task.Create(category, status, description);
        _taskRepository.Save(task);
    }
}

Entity:

public class Task
{
    //...

    public static void Create(Category category, Status status, string description)
    {
        return new Task
        {
            Category = category,
            Status = status,
            Description = descrtiption
        };
    }
}

I do it like this because I am consistently told that entities should not access the repositories, but it would make much more sense to me if I did this:

Entity:

public class Task
{
    //...

    public static void Create(Category category, string description)
    {
        return new Task
        {
            Category = category,
            Status = _statusRepository.GetById(Constants.Status.OutstandingId),
            Description = descrtiption
        };
    }
}

The status repository is dependecy injected anyway, so there is no real dependency, and this feels more to me thike it is the domain that is making thedecision that a task defaults to outstanding. The previous version feels like it is the application layeer making that decision. Any why are repository contracts often in the domain if this should not be a posibility?

Here is a more extreme example, here the domain decides urgency:

Entity:

public class Task
{
    //...

    public static void Create(Category category, string description)
    {
        var task = new Task
        {
            Category = category,
            Status = _statusRepository.GetById(Constants.Status.OutstandingId),
            Description = descrtiption
        };

        if(someCondition)
        {
            if(someValue > anotherValue)
            {
                task.Urgency = _urgencyRepository.GetById
                    (Constants.Urgency.UrgentId);
            }
            else
            {
                task.Urgency = _urgencyRepository.GetById
                    (Constants.Urgency.SemiUrgentId);
            }
        }
        else
        {
            task.Urgency = _urgencyRepository.GetById
                (Constants.Urgency.NotId);
        }

        return task;
    }
}

There is no way you would want to pass in all possible versions of Urgency, and no way you would want to calculate this business logic in the application layer, so surely this would be the most appropriate way?

So is this a valid reason to access repositories from the domain?

EDIT: This could also be the case on non static methods:

public class Task
{
    //...

    public void Update(Category category, string description)
    {
        Category = category,
        Status = _statusRepository.GetById(Constants.Status.OutstandingId),
        Description = descrtiption

        if(someCondition)
        {
            if(someValue > anotherValue)
            {
                Urgency = _urgencyRepository.GetById
                    (Constants.Urgency.UrgentId);
            }
            else
            {
                Urgency = _urgencyRepository.GetById
                    (Constants.Urgency.SemiUrgentId);
            }
        }
        else
        {
            Urgency = _urgencyRepository.GetById
                (Constants.Urgency.NotId);
        }

        return task;
    }
}
share|improve this question

4 Answers 4

up vote 6 down vote accepted

You are intermixing

entities should not access the repositories

(which is a good suggestion)

and

the domain layer should not access the repositories

(which might be bad suggestion as long as your repositories are part of the domain layer, not the application layer). Actually, your examples show no case where an entity accesses a repository, since you are using static methods which don't belong to any entity.

If you don't want to put that creation logic into a static method of the entity class, you can introduce separate factory classes (as part of the domain layer!) and put the creation logic there.

EDIT: to your Update example: given that _urgencyRepository and statusRepository are members of the class Task, defined as some kind of interface, you now need to inject them into any Task entity before you can use Update now (for example in the Task constructor). Or you define them as static members, but beware, that could easily cause multi threading problems, or just problems when you need different repositories for different Task entities at the same time.

This design makes it a little bit harder to create Task entities in isolation, thus harder to write unit tests for Task entities, harder to write automatic tests depending on Task entities, and you produce a little bit more memory overhead, since every Task entity now needs to hold that two references to the reposities. Of course, that may be tolerable in your case. On the other hand, creating a separate utility class TaskUpdater which keeps the references to the right repositories may be often or at least sometimes a better solution.

The important part is: TaskUpdater will be still part of the domain layer! Just because you put that update or creation code in a separate class does not mean you have to switch to another layer.

share|improve this answer
    
I have edited to show this applies to non-static methods as much as static ones. I never really thought of the factory method not being part of an entity. –  Paul T Davies Sep 24 '12 at 16:25
    
@PaulTDavies: see my edit –  Doc Brown Sep 24 '12 at 16:59
    
I agree with what you're saying here, but I'd add a concise piece drawing the point that Status = _statusRepository.GetById(Constants.Status.OutstandingId) is a business rule, one you could read as "The business dictates the initial status of all tasks will be Outstanding" and this is why that code line does not belong inside a repository, whose only concerns are data management via CRUD operations. –  Jimmy Hoffa Sep 24 '12 at 18:38
    
@JimmyHoffa: hm, no one here was suggesting to put that kind of line into one of the repository classes, neither the OP nor me - so what's your point? –  Doc Brown Sep 25 '12 at 5:42
    
I quite like the idea of the TaskUpdater as a domian service. It somehow seems like a little bit of a fudge just to retain DDD principles, but it does mean that I can avoid injecting the repository everytime I use Task. –  Paul T Davies Sep 25 '12 at 8:00

I have been trying to solve the same problem for sometime, I decided I want to be able to call Task.UpdateTask() like that, although I would rather it would be domain specific, in your case maybe I would call it Task.ChangeCategory(...) to indicate an action and not just CRUD.

anyways, I tried your problem and came up with this... have my cake and eat it too. The idea is that actions take place on the entity but without injection of all the dependencies. Instead work is done in static methods so they can access state of the entity. The factory puts its all together and normally will have everything it needs to do the work the entity needs to do. Client code now looks clean and clear and your entity is not dependent on any repository injection.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace UnitTestProject2
{
    public class ClientCode
    {
        public void Main()
        {
            TaskFactory factory = new TaskFactory();
            Task task = factory.Create();
            task.UpdateTask(new Category(), "some value");
        }

    }
    public class Category
    {
    }

    public class Task
    {
        public Action<Category, String> UpdateTask { get; set; }

        public static void UpdateTaskAction(Task task, Category category, string description)
        {
            // do the logic here, static can access private if needed
        }
    }

    public class TaskFactory
    {      
        public Task Create()
        {
            Task task = new Task();
            task.UpdateTask = (category, description) =>
                {
                    Task.UpdateTaskAction(task, category, description);
                };

            return task;
        }

    }
}
share|improve this answer

This is one reason I don't use Enums or pure lookup tables within my domain. Urgency and Status are both States and there is logic associated with a state that belongs with the state directly (e.g. what states can I transition to given my current state). Also, by recording a state as a pure value you lose information like how long the task was in a given state. I represent statuses as a class hierarchy like so. (In C#)

public class Interval
{
  public Interval(DateTime start, DateTime? end)
  {
    Start=start;
    End=end;
  }

  //To be called by internal framework
  protected Interval()
  {
  }

  public void End(DateTime? when=null)
  {
    if(when==null)
      when=DateTime.Now;
    End=when;
  }

  public DateTime Start{get;protected set;}

  public DateTime? End{get; protected set;}
}

public class TaskStatus
{
  protected TaskStatus()
  {
  }
  public Long Id {get;protected set;}

  public string Name {get; protected set;}

  public string Description {get; protected set;}

  public Interval Duration {get; protected set;}

  public virtual TNewStatus TransitionTo<TNewStatus>()
    where TNewStatus:TaskStatus
  {
    throw new NotImplementedException();
  }
}

public class OutStandingTaskStatus:TaskStatus
{
  protected OutStandingTaskStatus()
  {
  }

  public OutStandingTaskStatus(bool initialize)
  {
    Name="Oustanding";
    Description="For tasks that need to be addressed";
    Duration=new Interval(DateTime.Now,null);
  }

  public override TNewStatus TransitionTo<TNewStatus>()
  {
    if(typeof(TNewStatus)==typeof(CompletedTaskStatus))
    {
      var transitionDate=DateTime.Now();
      Duration.End(transitionDate);
      return new CompletedTaskStatus(true);
    }
    return base.TransitionTo<TNewStatus>();
  }
}

The implementation of CompletedTaskStatus would be pretty much the same.

There are several things to note here:

  1. I make the default constructors protected. This is so the framework can call it when pulling an object from persistence (both EntityFramework Code-first and NHibernate use proxies that are derived from your domain objects to do their magic).

  2. Many of the property setters are protected for the same reason. If I want to change the end date of an Interval, I have to call the Interval.End() function (this is part of Domain Driven Design, providing meaningful operations rather than Anemic Domain Objects.

  3. I don't show it here but the Task would likewise hide the details of how it stores its current status. I usually have a protected list of HistoricalStates that I allow the public to query if they're interested. Otherwise I expose the current state as a getter that queries HistoricalStates.Single(state.Duration.End==null).

  4. The TransitionTo function is significant because it can contain logic about which states are valid for transition. If you just have an enum, that logic has to lie elsewhere.

Hopefully, this helps you understand the DDD approach a little better.

share|improve this answer
    
This would certainly be the correct approach if the different states have different behaviour as in your state pattern example, and it certainly solves the issue discussed too. However, I would find it hard to justify a class for each state if they just had different values, not different behaviour. –  Paul T Davies Sep 26 '12 at 7:56

I don't know if your status example is real code or here just for the sake of demonstration, but it seems odd to me that you should implement Status as an Entity (not to mention an Aggregate Root) when its ID is a constant defined in code - Constants.Status.OutstandingId. Doesn't that defeat the purpose of "dynamic" statuses you can add as many of as you want in the database ?

I'd add that in your case, the construction of a Task (including the job of getting the right status from StatusRepository if need be) might deserve a TaskFactory rather than staying in the Task itself, since it is a non-trivial assemblage of objects.

But :

I am consistently told that entities should not access the repositories

This statement is imprecise and oversimplistic at best, misleading and dangerous at worst.

It's quite commonly accepted in domain-driven architectures that an entity shouldn't know how to store itself - that's the persistance ignorance principle. So no calls to its repository to add itself to the repository. Should it know how (and when) to store other entities ? Again, that responsibility seems to belong in another object - maybe an object that is aware of the execution context and overall progress of the current use case, like an Application layer service.

Could an entity use a repository to retrieve another entity ? 90% of the time it shouldn't have to, since the entities it needs are usually in the scope of its aggregate or obtainable by traversal of other objects. But there are times when they aren't. If you take a hierarchical structure, for instance, entities often need to access all their ancestors, a particular grandchild, etc. as part of their intrinsic behavior. They don't have a direct reference to these remote relatives. It would be inconvenient to pass these relatives around to them as parameters of the operation. So why not use a Repository to get them - provided they are aggregate roots ?

There are a few other examples. The thing is, sometimes there's behavior you can't place in a Domain service since it seems to fit perfectly in an existing entity. And yet, this entity needs to access a Repository to hydrate a root or a collection of roots that can't be passed to it.

So accessing a Repository from an Entity is not bad in itself, it can take different forms that result from of a variety of design decisions ranging from catastrophic to acceptable.

share|improve this answer
    
I disagree that an entity should use a repository to access an entity that it already has a relationship to - you should be able to traverse the object graph to access that entity. Using the repository in this way is an absolute no no. What I'm discussing here is entites that the entity does not yet have a reference to, but needs to create one under some business condition. –  Paul T Davies Sep 25 '12 at 7:58
    
Well if you've read me well, we totally agree on that... –  guillaume31 Sep 25 '12 at 9:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.