Domain modelling: Doing it right

Question

Having seen Jimmy Bogard's excellent video on crafting wicked domains, I tried to apply the same principles to one of my existing projects to evaluate how well i have grasped the concept. I have my queries and doubts listed below.

Domain Background: Admin can view a list of companies. He then approves a company. The database is supposed to update a Boolean field to true and store the id of the user who approved the company.

Initially, I had the following code written in my service layer. It passes the request to the repository which updates the appropriate fields in db and then sends a mail notification.

public void ApproveCompany(int companyId, int userId)
{
    _companyRep.ApproveCompany(companyId, userId);

    //send mail to company representatives on successful approval. 
}

Re factoring to create rich domains and encapsulate logic within the domain class, I created the below.

public void ApproveCompany(int companyId, int userId)
    {
        var user = _userRep.GetById(userId);
        var company = _companyRep.GetById(companyId);
        user.Approve(company);

        _companyRep.Insert(c);

        //send mail to company representatives on successful approval. 
    }

public class AdminUser
    {
        public string Name { get; set; }

        public void Approve(MyApprovedCompany c)
        {
            c.SetIsApproved(this);
        }       
    }

public class Company 
    {
        public bool IsApproved { get; private set; }
        public AdminUser ApprovedBy { get; private set; }

        public void SetIsApproved(AdminUser user)
        {
            if (this.IsApproved)
                throw new Exception("This company has already been approved by user: " + user.Name);

            this.IsApproved = true;
            this.ApprovedBy = user;
        }
    }

Queries:

1. Is what I have done completely correct/partially correct?
2. From a performance viewpoint is fetching the two objects just to create the proper class instances going to be a problem in the future?
3. Does the mail notification belong to the service layer?
4. How would my company repository look like to handle the property related to the user who approves the company? Should my company repository have a reference to the user repository (i think that is wrong).

Alternatively, I could write the service layer as below, but I don't think that is correct either.

public void ApproveCompany(int companyId, int userId)
{
    var user = _userRep.GetById(userId);
    var company = _companyRep.GetById(companyId);

    if(company.IsApproved)
    {
        throw new Exception("This company has already been approved by user: " + _userRep.GetById(company.ApprovedUserId).Name);        
    }
    else
    {
        user.Approve(company);
        _companyRep.Insert(c);
    }
}

Answer 1

These kind of questions are nearly impossible to answer correctly, however here's what I can tell you from what I see:

1. It's somewhat correct, but I usually favor placing behavior on the AR targeted by the operation rather than the actor performing it. Double-dispatching doesn't bring anything useful in this case IMHO. Therefore, I would simplify to company.Approve(adminUser). You might say that adminUser.approve(Company) better reflects a use case like "An admin user approves a company", but you could just turn it around and say that "A company is approved by an admin user". Also note that the company.SetIsApproved method you had is very CRUD oriented and certainly doesn't reflect your ubiquitous language very well.

2. ARs should be designed as small as possible. As long as you aren't creating unnecessary large cluster aggregates I don't see this becoming an issue. I strongly advise you to read Effective Aggregate Design by Vaughn Vernon.

3. Ideally, you should rely on domain events to implement the side-effects of an operation. There is plenty of information about how to implement domain events on the Web. However, with the lack of a publish/subscribe mechanism, it could be done in the application service layer or you could inject the mailing service at the AR method level.

4. The problem is that you are referencing an AR within another AR. ARs should usually reference other ARs by identity. Therefore, Company wouldn't hold onto AdminUser, only on the user's ID. By doing this, your problem goes away and you reduce the size of your AR.