Moving business logic towards the domain object as a design pattern

Question

My project is composed of several parts: Repositories, Services, Domain objects, etc in a .NET MVC web application.

My repositories handle all data i/o, the services are responsible for CRUD operations as well as other logic, such as getting a count of objects from the database.

I am trying to shift from an "anemic" domain model where my domain objects are simply classes with properties to having more business logic where it belongs. In this case, I would like to have the domain object handle the logic where it checks the database for duplicates, but I also don't want to have the code that communicates with the repository inside of the domain object (I read it is bad practice to have an object responsible for persistence with the database, so I'm assuming this is the same thing.)

I don't particularly like this solution, as it just doesn't feel good, and I'd like to get a better handle on how to handle these sorts of situations. Can anybody give me some advice here?

public interface IDuplicateChecker<T> where T : class
{
    bool IsDuplicate(T t);
}

public class DuplicateDocumentChecker : IDuplicateChecker<Document>
{
    public bool IsDuplicate(Document doc)
    {
        IDocumentService documentService = new DocumentService();

        // simplified check -- really there are a few other properties to match
        if (documentService.Count(doc.DocumentNumber) > 0)
        {
            return true;
        }

        return false;
    }
}

public class Document 
{
    public Document(string documentNumber)
    {
         // validation and other removed for brevity
         DocumentNumber = documentNumber;
    }

    // simplified document properties for brevity
    public string DocumentNumber { get; set; }
    public bool IsDuplicate { get; set; }

    public void CheckForDuplicates(IDuplicateChecker<Document> checker)
    {
        IsDuplicate = checker.IsDuplicate(this);
    }
}

Answer 1

First, I would like to say thank you for asking this question, because this is the type of situation that average enterprise devs face every day and being able to recognize that something doesn't "feel" right even though all the rules are followed is a good indicator that you're on the right track for figuring out what is going to be a good implementation and what isn't.

Getting to the code, from an overall perspective I would recommend the pattern of creating services in front of your domain layer, instead of your domain objects being fed into services that are dependencies of util/helper classes. An app layer can construct a domain Document object and pass it to a DocumentService, but encapsulating a DocumentService behind something that the app layer doesn't interact with could become hard to maintain.

A DocumentService could deal with saving the Document to the database, and as it's saving, it can then check for duplicates. Whether it checks for duplicates inline in the method or it calls another duplicate-checking object/util to do this is up to how big the method gets and how many different things the service is doing in the method.

Looking at your existing code, I'd recommend not getting too dogmatic about "one role, one responsibility--one class!" because it can be argued that documentService.SaveDocument(document) also has one responsibility (having to check for duplicates is superfluous--it's not part of the public contract of this method, although it may require a dependency or two passed in to the constructor).

I think when you have a DocumentService inside a very specific class used for a very specific purpose, it's a bit of a smell, because your DocumentService starts seeming like a bit of a Swiss army knife you start including inside all document-related objects, because hey, it's where document-related things go!

After all of that, if you implement according to my advice, your Document may remain a glorified property bag (though I would remove the properties like IsDuplicate from the Document. They don't belong on a domain entity. IsDuplicates should be refactored most simply to a boolean hasDuplicates or duplicatesExist as a local variable inside a method that has a responsibility to check for duplicates). And I think it's okay if it stays that way for now if your domain is not very fleshed out. It would be worse to put responsibilities/functionality on a domain object that didn't belong, because that's the type of problem that once is in place is really hard to break out because it's in such critical spot.

I would recommend focusing on your domain objects as smart entities first. That will help keep your mental model of them as a business object, but keeping the "smart" aspect in mind will allow you to apply "smart" logic to your objects. For example, if you have a one-to-many relationship between your objects, how does your domain track changes between these objects? If a child object assigns its parent reference, the parent object should have a reference to the child in its collection of children. So you might implement that by writing a "smart" setter so that when an Order assigns its Customer property, the Customer property adds it to its Orders collection if it doesn't exist. I think that's more in line with a domain object that is still an "entity" of sorts. Of course, there's definitely more that comes with the evolution of an application, but I'd start there.

For the time being, I'd focus on having intuitive services sitting in front of your domain, so the application layer can call these services, using domain objects to communicate by constructing them and passing them in to the services or having them returned from the services, or both.

Answer 2

• The Document object shouldn't be responsible for checking for duplicates. The object which will do any saving/updating should check if for a given Document a duplicate exists.

• The DuplicateDocumentChecker should maybe live in some kind of validation service.

But I will now focus on the provided code

public bool IsDuplicate(Document doc)
{
    IDocumentService documentService = new DocumentService();

    // simplified check -- really there are a few other properties to match
    if (documentService.Count(doc.DocumentNumber) > 0)
    {
        return true;
    }

    return false;
}

Here instead of checking .Count(condition) > 0 you should use .Any(condition). If you are using an IEnumerable which isn't an ICollection<T> nor an ICollection this will be faster.

For the IEnumerable the call to Count() will iterate over all items and returns the count of items. By a call to Any() the IEnumerable will get an Enumerator and will try to call MoveNext() and is returning true if this one method call will succeed.

If you have something like

if (condtion)
{
    return true;
}
return false;  

can be better expressed like

return condition;  

So your IsDuplicate() method can be refactored to

public bool IsDuplicate(Document doc)
{
    IDocumentService documentService = new DocumentService();
    // simplified check -- really there are a few other properties to match
    return documentService.Any(doc.DocumentNumber);
}

Answer 3

I am trying to shift from an "anemic" domain model where my domain objects are simply classes with properties to having more business logic where it belongs.

from a previous answer: The Document object shouldn't be responsible for checking for duplicates. But Document should define what a duplicate is. That is to say...

• override Equals() in Document
• New Class: DocumentCollection : List<Document>
  • Contains() will now find a duplicate
  • Find() can take a predicate (LINQ / anonymous method) and return a collection of duplicates.
  • Implement IEquality<T> if desired : Equals(Document,Document)
  • Encapsule the above so your client can be more expressive.
• This will eliminate the DuplicateDocumentChecker class
• MUCH better application of single responsibility principle.
• eliminates your troubling coupling
• While you're at it, also override ToString(). It can be as simple as returning the document number.
• If you're concerned about I/O due to very large collections of "heavy" objects then consider the Flyweight Design Pattern

.

public class Document { 
    public override bool Equals(object other) {
        if(other == null) return false;
        if(! other is Document) return false;
        if(this.DocumentNumber == other.DocumentNumber) return true;
        return false;
    }
}

public class DocumentCollection : List<Document> { 
    public bool IsDuplicate (Document thisDoc) { return this.Contains(thisDoc); }

    public bool Equals(Document a, Document b) { return a.Equals(b); }

    public bool Match(Document a, Document b) ( return this.Equals (a, b));

     public void RemoveAllDuplicates( DocumentCollection target )
    {
        this.RemoveAll( x => target.Contains( x ) );
    }

    // avoid duplicates up front
    // returning a bool might come in handy
     public bool AddDocument(Document aDoc) {
       if(aDoc == null) return false;
       if(this.Contains(aDoc)) return false;

       //the above could be more expressive as:
       if(IsDuplicate(aDoc)) return false;

       this.Add(aDoc);
       return true;
    }
}