Refactoring to Domain driven design

Question

I have a scenario am trying to refactor to DDD. I have a Batch which is an aggregate and List of BatchEntries. After a Batch is created and BatchEntries added, an SMS is sent to the individuals in the batch and the status of the batch changes from running to posted.

Any ideas on how to make the design better? The domain has two aggregates Batch and BatchEntry with Batch being the aggregate root.

The code looks like this

public class Batch : EntityBase, IValidatableObject
{
    public int BatchNumber { get; set; }
    public string Description { get; set; }
    public decimal TotalValue { get; set; }
    public bool SMSAlert { get; set; }
    public int Status { get; set; }

    private HashSet<BatchEntry> _batchEntries;
    public virtual ICollection<BatchEntry> BatchEntries
    {
        get{
            if (_batchEntries == null){
                _batchEntries = new HashSet<BatchEntry>();
            }
            return _batchEntries;
        }
        private set {
            _batchEntries = new HashSet<BatchEntry>(value);
        }
    }

    public static Batch Create(string description, decimal totalValue, bool smsAlert)
    {
        var batch = new Batch();
        batch.GenerateNewIdentity();
        batch.Description = description;
        batch.TotalValue = totalValue;
        batch.SMSAlert = smsAlert;
        return batch;
    }

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        // 
    }
}

public interface IBatchRepository : IRepository<Batch>
{
    int NextBatchNumber();
}

public class BatchEntry : EntityBase, IValidatableObject
{
    public Guid BatchId { get; set; }
    public virtual Batch Batch { get; private set; }
    public decimal Amount { get; set; }
    public Guid CustomerAccountId { get; set; }
    public virtual CustomerAccount CustomerAccount { get; private set; }

    public static BatchEntry Create(Guid batchId, Guid customerAccountId, decimal amount)
    {
        var batchEntry = new BatchEntry();
        batchEntry.GenerateNewIdentity();
        batchEntry.BatchId = batchId;
        batchEntry.CustomerAccountId = customerAccountId;
        batchEntry.Amount = amount;
        return batchEntry;
    }

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        //
    }
}

public interface IBatchEntryRepository : IRepository<BatchEntry>{}

The domain and domain services are exposed via Application Services. The Code in the application services is as follows:

//Application Services Code

public class BatchApplicationService : IBatchApplicationService
{
    private readonly IBatchRepository _batchRepository;
    private readonly IBatchEntryRepository _batchEntryRepository;

    public BatchAppService(IBatchRepository batchRepository, IBatchEntryRepository batchEntryRepository)
    {
        if (batchRepository == null) throw new ArgumentNullException("batchRepository");

        if (batchEntryRepository == null) throw new ArgumentNullException("batchEntryRepository");

        _batchRepository = batchRepository;
        _batchEntryRepository = batchEntryRepository;
    }

    public BatchDTO AddNewBatch(BatchDto batchDto)
    {
        if (batchDto != null)
        {
            var batch = Batch.Create(batchDto.Description, batchDto.TotalValue, batchDto.SMSAlert);
            batch.BatchNumber = _batchRepository.NextBatchNumber();
            batch.Status = (int)BatchStatus.Running;
            SaveBatch(batch);
            return batch.Map<BatchDto>();
        }
        else
        {
            //
        }
    }

    public bool UpdateBatch(BatchDto batchDto)
    {
        if (batchDto == null || batchDto.Id == Guid.Empty)
        {
            //
        }

        var persisted = _batchRepository.Get(batchDto.Id);
        if (persisted != null)
        {
            var result = false;
            var current = Batch.Create(batchDto.Description, batchDto.TotalValue, batchDto.SMSAlert);
            current.ChangeCurrentIdentity(persisted.Id);
            current.BatchNumber = persisted.BatchNumber;
            current.Status = persisted.Status;

            _batchRepository.Merge(persisted, current);
            _batchRepository.UnitOfWork.Commit();

            if (persisted.BatchEntries.Count != 0){
                persisted.BatchEntries.ToList().ForEach(x => _batchEntryRepository.Remove(x));
                _batchEntryRepository.UnitOfWork.Commit();
            }

            if (batchDto.BatchEntries != null && batchDto.BatchEntries.Any())
            {
                List<BatchEntry> batchEntries = new List<BatchEntry>();
                int counter = default(int);
                batchDTO.BatchEntries.ToList().ForEach(x =>
                {
                    var batchEntry = BatchEntry.Create(persisted.Id, x.CustomerAccountId, x.Amount);
                    batchEntries.Add(batchEntry);
                });
            }
            else result = true;
            return result;
        }
        else
        {
            //
        }
    }

    public bool MarkBatchAsPosted(BatchDto batchDto, int authStatus)
    {
        var result = false;
        if (batchDto == null || batchDto.Id == Guid.Empty)
        {
            //
        }

        var persisted = _batchRepository.Get(batchDto.Id);
        if (persisted != null)
        {
            var current = Batch.Create(batchDto.Description, batchDto.TotalValue, batchDto.SMSAlert);
            current.ChangeCurrentIdentity(persisted.Id);
            current.BatchNumber = persisted.BatchNumber;
            current.Status = authStatus;
            _batchRepository.Merge(persisted, current);
            _batchRepository.UnitOfWork.Commit();
            result = true;
        }
        else
        {
            //
        }
        return result;
    }

    private void SaveBatch(Batch batch)
    {
        var validator = EntityValidatorFactory.CreateValidator();
        if (validator.IsValid<Batch>(batch))
        {
            _batchRepository.Add(batch);
            _batchRepository.UnitOfWork.Commit();
        }
        else throw new ApplicationValidationErrorsException(validator.GetInvalidMessages(batch));
    }
}

Questions:

1. Where should the BatchStatus i.e Running, Posted be assigned?
2. Should the MarkBatchAsPosted method be defined as a mehod in the Batch Entity?
3. How best can this be redesigned for domain-driven design?

Answer 1

Although it looks simple, I'm not sure that I really understand your domain.

Statements such as

"After a Batch is created and BatchEntries added, an SMS is sent to the individuals in the batch and the status of the batch changes from running to posted"

makes very little sense to me. Can a batch really be a batch without any entries? If not, why would the batch automatically start when entries are added?

Anyway, I did not risk answering your 3 questions, but there's a few guidelines you seem to be violating and understanding them will allow you to come up with your own answers:

• Your domain is suffering from anemia.

• Non-root aggregates should not have their own repository because they should be accessed only through the root. Aggregate root's children should only be modified through their root (Tell Don't Ask). You should not have a BatchEntryRepository if EntryRepository is not a root.

• An aggregate root is a transactionnal boundary and only one should be modified in the same transaction. In addition, aggregate roots should be as small as possible, therefore you only keep the pieces needed to enforce invariants within the cluster. In your case, adding/removing batch entries seems to impact the Batch's status, so having a collection of BatchEntry under Batch makes sense and allows to protect invariants transactionnaly.

  Note: If there was a lot of contention on a Batch, e.g. multiple people working on the same Batch instance, adding and removing BatchEntry instances, then you might have to make BatchEntry it's own aggregate root and use enventual consistency to bring the system to a consistent state.

• Domain objects should usually be designed using an always-valid approach, meaning they can never be put in an invalid state. The UI should usually take care of validating user input to avoid sending incorrect commands, but the domain can just throw on you. Therefore, validator.IsValid<Batch>(batch) makes very little sense unless it is validating something the Batch couldn't possibly enforce by itself.

• Domain logic should not leak in application services and should usually be encapsulated in entities when possible (domain services otherwise). You are currently executing a lot of business logic in your application service e.g. if (persisted.BatchEntries.Count != 0){ ... }

• DDD is not CRUD. Using tactical DDD patterns in CRUD is not necessary wrong, but it's certainly not DDD. DDD is all about the ubiquitous language and modeling the domain. When you see methods named Update... or a tons of getter/setters, it usually means you are doing it wrong. DDD works best with task-based UI's which allows to focus on one business operation at a time. Your UpdateBatch method is doing way too much and should be segregated into more meaninful and granular business operations.

Hopefully my answer will help you refining your model, but I strongly advise you to read either Evans or Vernon... or both ;)