A Service Class Can Become a Procedural Junk Drawer

A web controller contained nearly two thousand lines of shipment creation. Moving those lines into a class described as a creation orchestrator sounded like an architectural improvement.

The new class contained nearly two thousand lines of shipment creation.

The first move was still useful. It removed the procedure from the HTTP adapter. What it did not do was make the procedure cohesive. Validation, payment gating, draft persistence, provider booking, tracking, document creation, response mapping, notifications, and indexing still changed inside one file.

Service and orchestrator are roles only when the code earns those names. Otherwise they are folders for procedures we no longer want in controllers.

Moving a procedure changes its address

The retained refactor moved roughly 1,900 lines from a controller into a new orchestrator. The diff added about as much code as it removed. That was a boundary change:

before
    HTTP controller
        parses request
        creates shipment
        books provider
        builds response

after
    HTTP controller
        delegates

    creation orchestrator
        creates shipment
        books provider
        builds response

The controller became narrower. Non-HTTP callers gained a place to enter the workflow. Those are real benefits.

Inside the moved procedure, however, the reasons to change remained mixed. A new payment rule, address-book rule, provider response, and notification could all edit the same class. Renaming the container did not decide which parts belonged together.

This is why line count alone is weak evidence. A 1,900-line parser may implement one grammar. A 200-line service may mix five unrelated policies. The important question is not how much code moved, but which changes still collide.

Read the procedure as a sequence of obligations

Long application procedures often look indivisible because local variables flow from top to bottom. Start by naming what the workflow promises, not by extracting arbitrary method-sized chunks.

The retained creation flow contained phases like these:

preflight
    resolve actor and cart
    check payment and booking eligibility

draft persistence
    create shipment, parties, parcels, and items
    synchronize selected address data

booking
    choose the provider path
    submit the booking
    translate booking failure

finalization
    persist tracking data
    create documents
    shape the response
    dispatch notifications and indexing

That map is more useful than a list of private methods. It gives each phase an input, an output, and a failure boundary.

The sequence still matters. Booking cannot precede draft creation. A tracking record needs a successful booking response. Some effects must wait until persistence succeeds. Decomposition should preserve those constraints rather than scatter them across listeners and hope ordering emerges.

Extract by change pressure, not by verb

The retained history did not split the procedure into equally sized pieces. It introduced roles as specific pressure became visible:

• preflight owned cart initialization, payment gating, and send eligibility;
• draft persistence owned transactional creation of the local record;
• address persistence owned party creation and address-book synchronization;
• booking owned provider gating, transfer, and booking failure handling;
• tracking owned persistence of returned tracking identifiers;
• document work owned proforma generation and dispatch;
• response work owned the caller-visible result;
• post-response effects owned cart updates, notifications, outbound logging, and indexing.

Some of those roles remained large. Size did not disqualify draft persistence, because its work shared a transaction and produced one durable draft. Address handling later separated when its own synchronization rules became a change pressure.

That order matters. Splitting every method on day one would have created a class-per-verb map without evidence that the pieces changed independently. The retained sequence used named phases first, then separated a nested concern when its boundary became clearer.

Orchestration is allowed to exist

A common overcorrection is to remove the coordinator entirely.

Once preflight, persistence, booking, and finalization have their own objects, the remaining class may look embarrassingly small:

final readonly class CreateShipment
{
    public function __construct(
        private Preflight $preflight,
        private PersistDraft $persistDraft,
        private BookDraft $bookDraft,
        private FinalizeBooking $finalizeBooking,
    ) {}

    public function execute(CreateShipmentCommand $command): ShipmentResult
    {
        $approved = $this->preflight->check($command);
        $draft = $this->persistDraft->execute($approved);
        $booking = $this->bookDraft->execute($draft);

        return $this->finalizeBooking->execute($draft, $booking);
    }
}

The small class has a real responsibility: own the workflow order and pass explicit results between phases.

Pushing that sequence into a controller would couple it to HTTP again. Hiding it behind events would make a required order implicit. Letting each phase call the next would turn reusable operations into a chain with no visible owner.

Coordination is cohesive when changes to the business sequence belong there. It becomes a junk drawer when every policy and side effect is implemented there too.

Shared state can conceal the remaining coupling

Extraction is easy to fake with mutable context:

$context->shipment = $drafts->create($context);
$context->booking = $bookings->book($context);
$context->response = $responses->build($context);

Each service can now read and mutate everything. The files are smaller, but the contracts remain implicit.

Prefer phase results which reveal what the next phase receives:

$approved = $preflight->check($command);
$draft = $drafts->create($approved);
$booking = $bookings->book($draft);
$result = $finalizer->finish($draft, $booking);

This is not an argument against every context object. A workflow with many stable values may benefit from one immutable context. The problem is a bag whose fields appear gradually and whose valid state depends on remembering which services have already run.

Ask of every extracted phase:

1. What must be true before it runs?
2. What new fact does it produce?
3. Which durable effects does it own?
4. How does failure stop or compensate the sequence?
5. Can a caller invoke it without constructing unrelated framework state?

If those answers still point to the original giant object, the extraction is mostly visual.

Dependencies reveal mixed roles, but do not prove them

A long constructor often makes the pressure visible. Repositories, provider clients, payment policy, mail, document generation, indexing, and framework context in one class suggest several collaborations.

Counting dependencies is not a design rule. An application coordinator may legitimately call several narrow roles. A report builder may need multiple read sources while still producing one report.

Classify dependencies by phase instead:

The table is a hypothesis. Confirm it with callers, transaction scope, failure handling, and history. Two dependencies used together because of framework convenience are weaker evidence than two files repeatedly changing for the same business rule.

Preserve behavior before improving the pieces

The retained extractions were described as behavior-preserving. That is the right first constraint for a workflow whose failures can leave partial durable state.

Characterize the observable sequence before moving it:

rejected preflight
    no draft and no provider call

draft failure
    no booking

booking failure
    known local failure state

booking success
    tracking persisted before response

finalization failure
    explicit recovery or reported partial state

Then move one phase without changing its policy. A refactor which also changes payment rules, retry behavior, and notification timing cannot tell you whether the new boundary preserved anything.

After the boundary holds, improve the phase with focused evidence. The goal is not merely more classes. It is the ability to change a payment rule without loading provider response mapping, or change an email rule without entering the draft transaction.

The later code is part of the evidence

The retained sequence ended with an orchestrator of roughly 144 lines and four methods. That does not prove the architecture stayed that way.

A later lineage again contains a large creation service with many workflow concerns. The history available here does not establish whether that happened through a replacement, merge, new requirements, incomplete adoption, or a deliberate reversal. It would be dishonest to present the extraction as a permanent cleanup or a measured success.

It does establish a usable method:

1. a wholesale move narrowed the delivery boundary but preserved the internal change surface;
2. named workflow phases made the sequence visible;
3. later commits extracted roles with distinct policies and effects; and
4. one small orchestrator retained ownership of order.

Architecture is not the state of one celebratory commit. If the same class starts accumulating policies again, the phase map is still useful for the next change.

Prove the ownership map, not a class count

The public reproduction models four change requests against three structures: a controller procedure, the same procedure moved wholesale into a service, and an orchestrator with four phase owners.

It proves that the wholesale move changes the file name but leaves all four changes on one implementation surface. The phase design assigns each example change to a different owner while retaining one coordinator for order.

The fixture does not measure cohesion, defects, development speed, or the private workflow. Retained diffs establish the move and extraction sequence, but not an enduring outcome.

A service class is useful when its name predicts which changes belong inside it. When the answer is “anything that happens during shipment creation,” the name describes a use case, not a cohesive implementation. Keep the use-case entry point. Move its independently changing obligations behind contracts the entry point can actually coordinate.