InvoiceService shouldn't exist

Nicolas Ruiz — Fri, 25 Sep 2026 07:00:00 +0000

We were creating invoices in two places.

One client came in through the API: the HTTP request arrived with an Input (a DTO), the required fields were validated, and the invoice was created cleanly.

Another flow came in through Kafka: a message arrived, a consumer called InvoiceService, and the invoice was created… without the DTO, without the validation. The required fields? Nobody checked them on that side.

HTTP (API)                     Kafka (consumer)
    │                               │
Input (DTO)                    raw message
required fields ✓              (nothing checks) ✗
    │                               │
Processor                      InvoiceService
    │                               │
    └─────► create an invoice ◄─────┘

Two front doors, two pieces of code, the same intent: "create an invoice". Except the rule "these fields are required" only lived on one side.

The result: depending on which door you came through, we created invoices with fields missing — and then it was the database that pulled us up: customer_id NOT NULL, constraint violation, boom.

The database protected the rule. Our code didn't.

The culprit isn't Kafka, nor the DTO. It's that "create an invoice" is a method on InvoiceService instead of being Invoice itself.

That's what I want to talk about.

My argument fits in one line: a business rule belongs to the object it protects.

The service that hosts it in its place should not exist — I mean the business catch-all service.

I'm not going to talk about clean architecture or DDD, because I know some devs will roll their eyes :D

And I'm not saying I don't make the mistake myself: it took me years to ask myself "why does code turn into spaghetti so fast?".

It isn't simple. But it should be.

Those famous services

We run into them every day:
PaymentService, OrderService, CustomerManager, ClientManager, and so on.

Just reading the name of the class, if you don't know what it does, that's already a smell.

We use this kind of class to put business rules in.
Because we're used to doing it that way, and very few projects do otherwise, really. That's how we learned, and we keep working on projects built like that.

I'm pointing at these services, but why?

How many times have we ended up with a class of 1000+ lines?
It makes DB calls, HTTP calls, it reaches into several domain models (entities?), it has if else everywhere...

The service becomes a pain to test, because you have to mock more than necessary to test a business rule.

One service can call another — spaghetti guaranteed.

Business rules have no home, let's admit it.

"Don't forget to call service->x, otherwise the calculation won't be consistent, so it'll be buggy" ¯_(ツ)_/¯.

Here's what a catch-all service looks like.
First, the constructor:

final class InvoiceService
{
    public function __construct(
        private readonly InvoiceRepository $invoices,
        private readonly CustomerRepository $customers,
        private readonly TaxApiClient $taxApi,
        private readonly EntityManagerInterface $em,
        private readonly LoggerInterface $logger,
    ) {}

Five dependencies: two repositories, an external API, the ORM, the logger. This class touches everything — and its tests often have to mock a good chunk of it.

A first method. With our business rules.

    public function applyDiscount(int $invoiceId, int $amount, string $reason): void
    {
        $invoice = $this->invoices->find($invoiceId);
        if ($invoice === null) {
            throw new InvoiceNotFoundException($invoiceId);
        }

        if ($invoice->getStatus() === 'issued') {
            throw new \LogicException('Invoice already issued, cannot be modified');
        }
        if ($amount <= 0) {
            throw new \InvalidArgumentException('Invalid discount');
        }

        $customer = $this->customers->find($invoice->getCustomerId());
        if ($customer->getType() === 'vip' && $amount > 100_000) {
            $amount = 100_000;    // VIP cap drowned in here
        }

Three rules that could belong to objects like Invoice or Discount, but that live here, nice and warm:

issued invoice untouchable
positive discount
VIP cap

Let's keep going, for fun:

        $line = new InvoiceLine();
        $line->setLabel('Discount: ' . $reason);
        $line->setAmount(-$amount);

        $invoice->getLines()->add($line);

        $total = 0;
        foreach ($invoice->getLines() as $l) {
            $total += $l->getAmount();
        }

We add a line, and we have to remember to recalculate the total by hand.

And the end:

        $vat = $this->taxApi->computeVat($total, $customer->getCountry());
        $invoice->setTotal($total);
        $invoice->setVatAmount($vat);

        $this->em->flush();
        $this->logger->info('Discount applied', ['invoice' => $invoiceId]);
    }

    // ... + 900 more lines of the same kind
    // (createInvoice, sendInvoice, refund, exportPdf...)
}

Phew, we didn't forget to set the total or the VAT.

This code works. It even passes the tests — well, if you manage to write them.

We've all seen this code, right?

So, what's the problem?

Nothing in Invoice protects its state. The rules live in the service, and all it takes is not going through it.

Six months later, somebody adds a refund:

// RefundInvoiceHandler, added by somebody else
public function __invoke(RefundInvoice $message): void
{
    $invoice = $this->invoices->find($message->invoiceId);

    $line = new InvoiceLine();
    $line->setLabel('Refund');
    $line->setAmount(-$message->amount);
    $invoice->getLines()->add($line);

    $this->em->flush();
}

It doesn't go through InvoiceService. It doesn't even know the rules live there. Result:

the invoice was already issued? We modify it anyway.
the total in the database? Still the old one.
the VAT? Same.

Nothing crashes, the tests pass. It's the customer who notices, reading their invoice.

And then the great classic — a service that calls another service:

final class PdfService
{
    public function __construct(
        private readonly InvoiceService $invoiceService, // -> smells strongly here
        private readonly PdfRenderer $renderer,
    ) {}

    public function generate(int $invoiceId): string
    {
        $invoice = $this->invoiceService->getInvoice($invoiceId);
        // if we forget this call, the PDF shows the old total
        $this->invoiceService->recalculateTotal($invoice);

        return $this->renderer->render($invoice);
    }
}

And be careful: everybody does this.

Good, not good?

Why we do this

I have a hypothesis: the MVC pattern, which everyone learns while studying.
Over time, the Model became anemic[1], a 1-1 with a database table.

A big bag of data.

And even more so with the arrival of frameworks (Symfony, Spring Boot, and so on)

We end up with plenty of getters and setters in a class that does nothing...

Why add behaviour to it?

So we first put the rules in the controllers, which orchestrate.

Then these controllers grow as the number of routes grows, along with the business rules we keep adding.
And unit testing them gets complicated, because you cross every layer: HTTP, domain, DB, and so on.

A new layer appeared: the "service". To "split things up".

That's all very nice, but how do we actually do it?

Let's start by giving our objects their responsibilities back.
What is the definition of an object?

An object is an instance of a class.

That's what we're taught, and technically it's fine.

But what is an object for?

It's as if I asked: "what is a car?"
And the answer came back: it's a mechanical assembly of several parts, like wheels, an engine, and so on.

So what is an object for?

According to Alan Kay, who coined the term "object-oriented" (email to the squeak-dev list, October 1998):

The big idea is messaging.

So it isn't inheritance, the instance, the class, and so on.

It's the message!

An object responds to messages, while protecting the consistency of its state.

We don't ask it for its data to decide in its place what it should do.
We ask it to do the thing directly.

Tell, don't ask.[2]

A (domain) object is not a bag of data (entities, we can see you ;)

Example: instead of reading the state, then deciding what to do (like here)

class Service
// ...
if ($invoice->getStatus() === 'issued') {
    // ... do something
}
$invoice->setStatus('issued'); // don't think, take this
$invoiceRepository->save($invoice);

We can make the object speak.

And the behaviour lives in the object, not in a service:

class Invoice
{
    private string $status;

    public function isIssued(): bool
    {
        return $this->status === 'issued';
    }

    public function markAsIssued(): void
    {
        if ($this->isIssued()) {
            // the object refuses the inconsistent state
            throw new \DomainException('Invoice already issued');
        }
        $this->status = 'issued';
    }
}

There's something essential to understand here.

Invoice isn't just a 1-1 with the invoice table… (or not at all).

The object becomes responsible for its own rules again; it protects its state and refuses what is inconsistent.

"Okay, but the object is simple here. My objects have relations, sub-objects…"

Thankfully!
Let's imagine InvoiceLine now.

When the object has sub-objects

Invoice keeps its lines and protects the rule that binds them. Concretely:

the list of lines is encapsulated: nobody can add a line outside of this class;
addLine() is the only door, and it recalculates the total every time;
no setter[3], not even setTotal(): impossible to knock the total and the lines out of sync.

final class Invoice
{
    private string $status = 'draft';
    private int $total = 0;
    /** @var Collection<int, InvoiceLine> */
    private Collection $lines;

    // PRIVATE constructor: nobody builds an Invoice "by hand"
    private function __construct(
        private Uuid $id,
        // required: no invoice without a customer
        private int $customerId,
    ) {
        $this->lines = new ArrayCollection();
    }

    // the only "way in" to create an invoice
    // — whatever the source (HTTP, Kafka, cli …)
    public static function create(int $customerId): self
    {
        return new self(Uuid::v4(), $customerId);
    }

    public function id(): Uuid
    {
        return $this->id;
    }

A private constructor, one factory: Invoice::create(), or nothing. No invoice without a customer, whichever door you came through.
And identity is born with the object: no need to wait for the database to have an id.

    // the ONLY way to add a line → impossible to bypass the recalculation
    public function addLine(string $label, int $amount): void
    {
        if ($this->isIssued()) {
            throw new \DomainException('Invoice issued: lines cannot be modified');
        }

        $this->lines->add(new InvoiceLine($label, $amount));

        // the "total = sum" invariant is guaranteed HERE
        $this->recalculateTotal();
    }

Invoice checks the state, adds, recalculates — it forgets nothing.

    private function recalculateTotal(): void
    {
        $this->total = 0;
        foreach ($this->lines as $line) {
            $this->total += $line->amount();
        }
    }
}

Result: adding a line and forgetting the recalculation becomes impossible. Invoice and its lines form what we call
an aggregate, with Invoice as its root.

HTTP, Kafka, CLI, any input at all: they all go through create(), they all get the same rule.

The bug from the very beginning — the validation only lived on one side. Remember?

Here it lives in the object: no more incomplete invoice that's going to blow up on a NOT NULL.

"But we could have fixed that differently: validate the message on the consumer side too, or make both doors go through the same use case."

True. Except that if the rule lives in the front doors, you have to remember to reproduce it everywhere — HTTP, Kafka, CLI… And those validations will end up diverging at the next change.

The private constructor, on the other hand, is not a convention: it's a constraint. The invalid state is no longer representable.

What's left for the service

Okay… but then where do we put this code?

I mentioned it above without meaning to: "use case".

Depending on conventions and teams, people talk about a UseCase or an Application Service (careful:
this is NOT the layer we want to kill).

Its only job is orchestration.

Example:

final class CreateInvoice
{
    public function __construct(
        // a PORT (interface), not the DB
        private readonly InvoiceRepository $invoices,
        private readonly CustomerRepository $customers,
        // to announce the fact
        private readonly EventDispatcher $events,
    ) {}

    public function __invoke(CreateInvoiceCommand $command): void
    {
        $customer = $this->customers->get($command->customerId);
        $invoice = Invoice::create($customer->id());

        foreach ($command->lines as $line) {
            // it's the AGGREGATE that decides
            $invoice->addLine($line->label, $line->amount);
        }

        // ...

        $this->invoices->save($invoice);

        // domain fact, in the PAST tense
        $this->events->dispatch(new InvoiceCreated($invoice->id())); // we should use the outbox pattern here
    }
}

For the record: CreateInvoice announces a fact; it does NOT send the email itself. Elsewhere, a handler reacts to the past event — and that is infrastructure:

final class SendInvoiceEmail
{
    public function __invoke(InvoiceCreated $event): void
    {
        // ... we send the email
    }
}

Tomorrow an SMS, an accounting notification? We add a handler, the use case doesn't move.[4]

The name

The smell from the beginning: "Service", "Manager"
If you can't name your class without "Service", "Manager" or "Helper", it's doing too much.

So we name it by what it does: InvoiceService becomes CreateInvoice, IssueInvoice, ApplyDiscount. One action, one class.

And the name becomes a signal: an email being sent inside CreateInvoice jumps out in review — which is exactly why it lives in SendInvoiceEmail.

It isn't a constraint like the private constructor: nothing stops you from doing it. But the catch-all can't grow in silence any more.

Can a use case call another use case?

If the question crosses your mind: no.

What we might picture doing:

final class IssueInvoice
{
    public function __construct(
        private readonly CreateInvoice $createInvoice,
        private readonly AddShippingFees $addShippingFees,
    ) {}

    public function __invoke(IssueInvoiceCommand $command): void
    {
        // one use case driving two others
        // and which had to return an id "just for this case"
        $invoiceId = ($this->createInvoice)(new CreateInvoiceCommand($command->customerId, $command->lines));

        ($this->addShippingFees)(new AddShippingFeesCommand($invoiceId, $command->shippingFees));
    }
}

Three things are wrong.

The id first: CreateInvoice returned nothing, it didn't need to. Now it does, and for a single caller. It no longer writes for the domain, it writes for another use case.
Transactions next: each use case commits its own and publishes its events.

So CreateInvoice commits, InvoiceCreated goes out, the email reaches the customer. Then AddShippingFees fails.

Too late: the invoice is in the database without its shipping fees, and the customer has already received the wrong amount. The first commit doesn't come back.
And above all, IssueInvoice becomes a conductor again, one that knows the details of the others. The catch-all we just pushed out the door comes back through the window, with use cases instead of methods.

So what do we do?

A business rule? It goes in the object: it protects its state and stays consistent.

A reaction to a fact that has already happened? That's a handler on the event, like SendInvoiceEmail.

And if the business wants to create the invoice and add the shipping fees in one go? A single use case, doing both itself: Invoice::create(), then addLine().

And if you have two use cases that really must follow one another, ask yourself the transaction question:

one and the same transaction? Then it's only one use case.
two transactions, two aggregates? Then the second one isn't called, it's triggered: the first emits its event, a handler takes over.

And there we step into another subject: outbox pattern[5], saga and compensation, eventual consistency. That deserves articles of its own.

To wrap up

What can we do to improve our project a little every day?
We're not going to do a big bang or refactor 1000 lines of a service.
That would be dangerous, and counter-productive.

The next time we have a business rule to write, we can ask the question.
Can this rule manage itself inside an object, or not?

If yes, great: we put it in there, and our existing services will call it.
That's fine, it's better than yesterday: the rule now exists in only one place.

If no, we can think: is there an object missing in the code, one that does exist in our business language?
A class isn't necessarily a table in the database, don't forget :D.

And little by little, our services will slim down, because responsibility will have moved.
Maybe they'll even disappear, replaced by classes of intent: CreateInvoice, IssueInvoice, ApplyDiscount.

We haven't talked about the cases where we fetch data just to display it.
Spoiler: no need for an aggregate or a use case for that — often, a simple query is enough[6].
(Yet another article!)

And the bonus, the one we've been waiting for since the beginning: that rule, now, you test it in four lines.

public function test_the_total_is_the_sum_of_the_lines(): void
{
    $invoice = Invoice::create(customerId: 42);   // the only way in
    $invoice->addLine('Service', 10000);         // €100.00
    $invoice->addLine('Shipping fees', 500);     //   €5.00

    // no DB, no HTTP, no mock
    self::assertSame(10500, $invoice->total());
}

At the start of the article, testing that rule meant mocking the DB, the HTTP client, the entire planet.
Now? Four lines.

And the invoice without a customer, the one that blew up on a NOT NULL depending on the door it came through?
It is no longer representable — not even through the door nobody has imagined yet.

Now it's our code that protects the rule. The database has nothing left to tell us.

The full code

This article sticks to the essentials. Real life adds reverse charge between countries, issuing that freezes the invoice, a rate read from a table or through an API — everything that usually ends up in the catch-all.

I gave my article to Claude Code, which generated the project from it — aggregate, ports[7], use cases and tests included: nicolasrz/article-invoice-exemple.

I find it very representative of what I'm talking about.

Notes

[1] Martin Fowler gave this pitfall a name in 2003: Anemic Domain Model.

[2] A principle popularised by Andy Hunt and Dave Thomas, the authors of The Pragmatic Programmer. Martin Fowler sums it up here: TellDontAsk.

[3] "Wait, an entity without setters? Doctrine will never be able to load it back."

Sure it will. Doctrine hydrates through reflection: it writes straight into the private properties, without going through a single accessor — and it doesn't even call the constructor.

Public getters and setters aren't an ORM constraint. They're a habit, plus a make:entity that generates them by default.

[4] For the DDD purists: we could raise the event in the aggregate itself — Invoice records InvoiceCreated — then loop over its events after persistence to dispatch them. Not really the subject here.

[5] In CreateInvoice, we save and then dispatch. If the dispatch fails after the commit, the event is lost: the invoice exists, but nobody is told. The outbox pattern writes the event to a table, in the same transaction as the invoice; a separate process publishes it afterwards.

[6] Separating writes (aggregates, use cases) from reads (direct queries) has a name: CQRS, for Command Query Responsibility Segregation.

[7] A term from hexagonal architecture ("Ports and Adapters", Alistair Cockburn). The domain defines the interface, that's the port; the infrastructure provides the implementation, that's the adapter: Doctrine, an HTTP client, or an in-memory version for tests.

DEV Community: Nicolas Ruiz