Posts in this series:

As part of the red-green-refactor TDD process, the second step of making the test pass means we write the simplest (but still correct) code that can possibly work that flips our test from red to green. For me, this means in an OO language creating dumb, procedural code. I hardcode as much as I can, I don’t introduce any abstractions or indirections, and try to make it as boring as I can.

This means if I’m using some variant of MVC, everything goes in the controller action. Web APIs, put it in the action. Razor Pages? Your “OnGet/Post” methods. Even when using MediatR, I just dump everything in the handler.

I do this because I don’t want to assume the complexity of my code before it is written. I want to let the code smells guide my way into where the code should belong. If I try to “prefactor” the code, I’m probably wrong (I’m usually wrong, anyway).

In this sample application, I’ve got a persistence/data model representing a loyalty rewards system with members and offers:

Data model of Member, Offer, and OfferType classes

Data model of Member, Offer, and OfferType classes

 

Our data model has a Member, an Offer, and an OfferType. We have associations between our classes, and even the names of those associations represent the nature of the relationships, tracking to the terms the business uses.

When we assign an offer to a Member, we use the OfferType to calculate the expiration date, and the Value comes from an external API. The MediatR handler to do this looks like:

public class AssignOfferHandler : IRequestHandler<AssignOfferRequest>
{
    private readonly AppDbContext _appDbContext;
    private readonly HttpClient _httpClient;

    public AssignOfferHandler(
        AppDbContext appDbContext,
        HttpClient httpClient)
    {
        _appDbContext = appDbContext;
        _httpClient = httpClient;
    }

    public async Task<Unit> Handle(AssignOfferRequest request, CancellationToken cancellationToken)
    {
        var member = await _appDbContext.Members.FindAsync(request.MemberId, cancellationToken);
        var offerType = await _appDbContext.OfferTypes.FindAsync(request.OfferTypeId, cancellationToken);

        // Calculate offer value
        var response = await _httpClient.GetAsync(
            $"/calculate-offer-value?email={member.Email}&offerType={offerType.Name}",
            cancellationToken);

        response.EnsureSuccessStatusCode();

        await using var responseStream = await response.Content.ReadAsStreamAsync(cancellationToken);
        var value = await JsonSerializer.DeserializeAsync<int>(responseStream, cancellationToken: cancellationToken);

        // Calculate expiration date
        DateTime dateExpiring;

        switch (offerType.ExpirationType)
        {
            case ExpirationType.Assignment:
                dateExpiring = DateTime.Today.AddDays(offerType.DaysValid);
                break;
            case ExpirationType.Fixed:
                dateExpiring = offerType.BeginDate?.AddDays(offerType.DaysValid)
                               ?? throw new InvalidOperationException();
                break;
            default:
                throw new ArgumentOutOfRangeException();
        }

        // Assign offer
        var offer = new Offer
        {
            MemberAssigned = member,
            Type = offerType,
            Value = value,
            DateExpiring = dateExpiring
        };
        member.AssignedOffers.Add(offer);
        member.NumberOfActiveOffers++;

        await _appDbContext.Offers.AddAsync(offer, cancellationToken);

        await _appDbContext.SaveChangesAsync(cancellationToken);

        return Unit.Value;
    }
}

It’s quite a lot of code but it’s not huge. We do see that there are some code comments to split out some of the major sections. Roughly, the method breaks down to:

  1. Data access to load data models we care about
  2. Calculate offer value using external web API
  3. Calculate expiration date based on the OfferType
  4. Mutate our data models to assign an offer to our member
  5. Save our data to the database

Generally speaking, steps 1 and 5 are fairly universal in functions/features in our applications. We almost always need to load data, work with data, then save the data. However, it’s steps 2-4 that the real business logic lie.

So what’s wrong with this? The biggest code smell I see is Long Function (Method), where, well, my method is too long to fit on a screen. It’s long enough to introduce code comments to split up the main parts.

In the next post, we’ll look to see how to tackle this long method, eventually refactoring our code into the domain model.

Source: jimmybogard.com

Have a tech-oriented question? We'd love to talk.