Domain-Driven Refactoring: Encapsulating Data

Posts in this series:

• Intro
• Procedural Beginnings
• Long Methods
• Extracting Domain Services
• Defactoring and Pushing Behavior Down
• Encapsulating Data
• Encapsulating Collections

In the last post, we looked at using a few common refactorings to encapsulate operations and push behavior down into our domain model. While this works to encapsulate operations, we haven't yet encapsulated our data. Take a look at the current Offer model:

public class Offer : Entity
{
    public Member MemberAssigned { get; set; }
    public OfferType Type { get; set; }
    public DateTime DateExpiring { get; set; }
    public int Value { get; set; }
}

And our Member model:

public class Member : Entity
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Email { get; set; }
    public List<Offer> AssignedOffers { get; set; } = new List<Offer>();
    public int NumberOfActiveOffers { get; set; }

    public Offer AssignOffer(OfferType offerType, int value)
    {
       // implementation from before
    }
}

Yes, we have the AssignOffer method, but everything else about my object is completely open for modification. We can do this:

var member = new Member();
member.NumberOfActiveOffers = Int32.MaxValue;
member.AssignedOffers.Clear();

var offer = new Offer();
offer.Member.AssignedOffers = null; // this throws anyway

Our model is a data model and not a domain model because it still does not properly encapsulate behavior and data. Since I'm trying to create a domain model, that makes this model an anemic domain model. It contains data, but does not encapsulate operations that mutate that data.

But we can fix this, again with refactoring!

Encapsulating Simple Primitives

The simplest means of encapsulating data is to only allow mutation of that data inside that object. But that's easier said than done! Simply making a member private may break lots of usages/clients. So we'd like to take a more measured approach.

One easy way to see if making a member private is to simply change the access modifier and see what breaks. That's...OK...but we can do a bit better by examining the usages. On the Offer class, let's look for usages of the setters of its properties. We might find a lot of usages, in application code and in (non-existent) tests, but in our application we only find one:

var offer = new Offer
{
    MemberAssigned = this,
    Type = offerType,
    Value = value,
    DateExpiring = dateExpiring
};

These values are only set once, and they're part of construction. And for our model, this makes sense, because from the business perspective you're not allowed to change most of these values once the offer has been assigned. We only see one other usage of the DateExpiring value and it is for immediately expiring an offer.

So first, we'll need to encapsulate that operation of creating the Offer, which ReSharper has as a refactoring for us:

This creates a new constructor (technically two) in my model:

public Offer()
{
}

public Offer(Member memberAssigned, OfferType type, int value, DateTime dateExpiring)
{
    MemberAssigned = memberAssigned;
    Type = type;
    Value = value;
    DateExpiring = dateExpiring;
}

The first no-arg constructor is simply to ensure we don't break anyone. My calling code is automatically updated, too:

var dateExpiring = offerType.ExpirationType switch
{
    ExpirationType.Assignment => DateTime.Today.AddDays(offerType.DaysValid),
    ExpirationType.Fixed => offerType.BeginDate?.AddDays(offerType.DaysValid) ??
                            throw new InvalidOperationException(),
    _ => throw new ArgumentOutOfRangeException(nameof(offerType))
};

var offer = new Offer(this, offerType, value, dateExpiring);
AssignedOffers.Add(offer);
NumberOfActiveOffers++;

Looking at this code, we can now see that dateExpiring is only calculated to pass directly into the constructor, and is calculated by offerType. Nothing in this Member class uses those values, so we can safely inline that parameter in the Offer constructor:

And now our constructor becomes:

public Offer(Member memberAssigned, OfferType type, int value)
{
    MemberAssigned = memberAssigned;
    Type = type;
    Value = value;
    DateExpiring = type.ExpirationType switch
    {
        ExpirationType.Assignment => DateTime.Today.AddDays(type.DaysValid),
        ExpirationType.Fixed => type.BeginDate?.AddDays(type.DaysValid) ??
                                throw new InvalidOperationException(),
        _ => throw new ArgumentOutOfRangeException(nameof(type))
    };
}

The calling code removes all DateExpiring calculation, since this is now entirely the Offer class's responsibility:

public Offer AssignOffer(OfferType offerType, int value)
{
    var offer = new Offer(this, offerType, value);
    AssignedOffers.Add(offer);
    NumberOfActiveOffers++;
    return offer;
}

Not bad! Finally, we can mark members private that only have a single use in the constructor:

public class Offer : Entity
{
    private Offer()
    {
    }

    public Offer(Member memberAssigned, OfferType type, int value)
    {
        MemberAssigned = memberAssigned;
        Type = type;
        Value = value;
        DateExpiring = type.ExpirationType switch
        {
            ExpirationType.Assignment => DateTime.Today.AddDays(type.DaysValid),
            ExpirationType.Fixed => type.BeginDate?.AddDays(type.DaysValid) ??
                                    throw new InvalidOperationException(),
            _ => throw new ArgumentOutOfRangeException(nameof(type))
        };
    }

    public Member MemberAssigned { get; private set; }
    public OfferType Type { get; private set; }
    public DateTime DateExpiring { get; private set; }
    public int Value { get; private set; }
}

We also mark that constructor private, reserving only for EF Core to access.

One might argue that still including private constructors/setters does not make our domain model completely "persistent ignorant". I argue such efforts are largely a waste of time and energy.

We compile but oops! It turns out we still have a usage of one of these private members, so we need to make it public again. It's in a separate handler for expiring offers:

public class ExpireOfferHandler : IRequestHandler<ExpireOfferRequest>
{
    private readonly AppDbContext _appDbContext;

    public ExpireOfferHandler(AppDbContext appDbContext) => _appDbContext = appDbContext;

    public async Task<Unit> Handle(ExpireOfferRequest request, CancellationToken cancellationToken)
    {
        var member = await _appDbContext.Members
            .Include(m => m.AssignedOffers)
            .SingleOrDefaultAsync(m => m.Id == request.MemberId, cancellationToken);

        var offer = member.AssignedOffers.SingleOrDefault(o => o.Id == request.OfferId)
            ?? throw new ArgumentException("Offer not found.", nameof(request.OfferId));

        offer.DateExpiring = DateTime.Today;
        member.NumberOfActiveOffers--;

        await _appDbContext.SaveChangesAsync(cancellationToken);

        return Unit.Value;
    }
}

We have two options here:

• Leave the property public for another day
• Refactor this other handler to encapsulate behavior

Leaving for another day is boring, let's refactor this method too!

Pushing behavior down

This other handler is very similar to our first handler, where we have 3 steps:

• Load data
• Do some business logic
• Save data

Based on our experience from our previous handler, we can zero in on this code:

var offer = member.AssignedOffers.SingleOrDefault(o => o.Id == request.OfferId)
    ?? throw new ArgumentException("Offer not found.", nameof(request.OfferId));

offer.DateExpiring = DateTime.Today;
member.NumberOfActiveOffers--;

As a candidate to extract a method and push that method down. It looks like this code still mainly deals with our Member object, so let's do that. We'll:

• Extract Method
• Make Method Non-Static

This creates a new method, ExpireOffer on our Member class:

public void ExpireOffer(Guid offerId)
{
    var offer = AssignedOffers.SingleOrDefault(o => o.Id == offerId)
                ?? throw new ArgumentException("Offer not found.", nameof(offerId));

    offer.Expire();

    NumberOfActiveOffers--;
}

And an Expire method on Offer:

public void Expire()
{
    DateExpiring = DateTime.Today;
}

And our handler becomes quite a bit smaller:

public async Task<Unit> Handle(
    ExpireOfferRequest request, 
    CancellationToken cancellationToken)
{
    var member = await _appDbContext.Members
        .Include(m => m.AssignedOffers)
        .SingleOrDefaultAsync(m => m.Id == request.MemberId, cancellationToken);

    member.ExpireOffer(request.OfferId);

    await _appDbContext.SaveChangesAsync(cancellationToken);

    return Unit.Value;
}

Now that we've got our DateExpiring setter usage to inside our domain model, we can mark that setter private (as well as all the others we can).

Encapsulating Member

On the Member class, my example isn't completely flushed out but we can make up some requirements like:

• Members are required to have a name and email
• Members can change their name and email

With this in mind we can introduce a constructor and method:

private Member() { }

public Member(string firstName, string lastName, string email)
{
    FirstName = firstName ?? throw new ArgumentNullException(nameof(firstName));
    LastName = lastName ?? throw new ArgumentNullException(nameof(lastName));
    Email = email ?? throw new ArgumentNullException(nameof(email));
}

public string FirstName { get; private set; }
public string LastName { get; private set; }
public string Email { get; private set; }
public List<Offer> AssignedOffers { get; set; } = new List<Offer>();
public int NumberOfActiveOffers { get; set; }

public void UpdatePersonalInfo(string firstName, string lastName, string email)
{
    FirstName = firstName ?? throw new ArgumentNullException(nameof(firstName));
    LastName = lastName ?? throw new ArgumentNullException(nameof(lastName));
    Email = email ?? throw new ArgumentNullException(nameof(email));
}

Finally, we can mark that NumberOfActiveOffers property private as the only usages are this class.

With these refactorings in place, I'm introducing proper OO design by encapsulating both data AND behavior. And for me, that's really what DDD modeling is about - refactoring procedural code to more OO, with a design direction towards a domain model. It's still just fundamental OO, though.

In the next post, we'll look at options for encapsulating the collection.