What's Not To Like About This Code? An Experiment. - The Code Whisperer

Articles
1

December 20, 2022 Refactoring, Simple Design, What's Not To Like About This Code?

This is a companion discussion topic for the original entry at https://blog.thecodewhisperer.com/permalink/whats-not-to-like-about-this-code-1
2

I totally agree with the assessment, as well as with the equally painful ideas for working around it.

But what would be a healthy way to address this?

Have a domain class that encapsulates the logic: if <condition>: shipOrder() where ship order is emitting an event that is captured by a wrapper/port/adapter that knows that a shipOrder event should result in the sendLocalcall?

That would make the test and the domain more clear, but the added level of indirection makes it hard to reason about the whole order lifecycle…

The alternative is passing in an interface into the domain layer and then doing some integrated tests using mocks to ensure the right method is called, but that test then becomes an “implementation test” instead of a “requirement test”…

And this doubt is what creates the broken window situation, the reasoning goes “Hmm, this is bad, but I don’t have any good solutions yet. I will leave it as is and come back later”

It would be great to see how others would address this!

4

Interesting experiment to try.

If I ran the zoo, I would want clearer separation between the framework code and the bespoke domain logic.

Depending on local style and idioms, that might look like a pattern match, or it might look like a state machine that makes a parameterized choice between different alternatives that would be implemented in the adapter code shown here.

// WARNING: un-linted code ahead

static T DoTheRightThing<T>(Data, shipIt, noDontShipIt) {
    if (Data.IsOrderPlaced && Data.IsOrderBilled)
    {
        return shipIt(Data.orderId);
    } else {
        return noDontShipIt();
    }
}

With the client code responsible for providing the “factories” used to construct an answer and figuring out what to do with it.

In a toy problem, or in an example designed to focus attention on how to work with the framework, this is enough overkill to be inappropriate. But if the domain logic is much bigger or much more important than the framework code, the trade offs might warrant this separation.

(If this idea looks alien - add Cory Benfield’s “Building Protocol Libraries the Right Way” to your viewing list. Very roughly - the idea is to separate the re-usable logic from this specific implementation of the side effects).

Exact placement of the boundaries might depend on whether Data and ShipOrder are of the framework, of the messaging, or of the business.

5

Somewhat. The “implementation test” part comes from “we’ve chosen an event-based design for this” as opposed to something like Transaction Scripts.

If we separated the NServiceBus implementation from the business rules, then we’d have the best of both worlds: the “realism” of the event-based design with the free option to take either an integrated tests approach (with a lightweight service bus) or an isolated tests approach (using test doubles for the Abstract Service Bus). The Lightweight Service Bus that we choose could encapsulate the specific domain events for us completely, if we wanted to hide that detail completely and focus on the domain as a whole.

We also have the option of designing the individual handlers like so:

if (isOrderPlaced() && isOrderBilled()) { shipOrder() }

where we set an expectation on shipOrder(), or like so:

if (isOrderPlaced() && isOrderBilled()) { return new ShipOrderRequest(orderId()) }

where ShipOrderRequest is a value that represents the next step in the domain event chain. As far as I can tell, this is a free choice, since neither option makes it difficult to integrate into the NServiceBus-bound design.

I see it differently: what creates the broken window situation is not having the habit of actually coming back later. Having that habit makes the doubt harmless.

1 Like
6

I like this approach because it is flexible. It is another version of “set an expectation on shipOrder()”. We could easily Introduce a Parameter Object for (shipIt, noDontShipIt), then Extract Interface on the parameter object, and have a tiny interface/implementation split that makes testing easy without making it more difficult to integrate with NServiceBus.

Moreover, we could easily refactor that to

static DomainEvent DoTheRightThing(Data) {
    return (Data.IsOrderPlaced && Data.IsOrderBilled)
        ? new ShipOrderEvent(Data.OrderId)
        : new MarkOrderUnshippableEvent(Data.OrderId);
}

and then the function that turns DomainEvent into T is essentially a fold over the various domain event types, treating DomainEvent as a sum type. And maybe we don’t even need that function; maybe it’s enough to return the next event and let the state machine continue running.