Masoud Bahrami

A Look at a Subtle yet Impactful Code Smell

Intro

In software systems, clarity of intent and separation of responsibility are foundational to maintainable code. However, we often encounter situations where a single method, class, or component takes on more than one role. This is what I refer to as the Mixed Responsibility Smell.

At first glance, combining responsibilities may seem like a shortcut for efficiency or convenience, but it often leads to code that’s harder to understand, maintain, and evolve.

A Simple Example

Let’s begin with a simplified example from a financial context:

public Money Refund(Money amount)
{
    Balance += amount;
    return Balance;
}

This method does two things at once: it changes the internal state (by modifying the balance) and also returns the new state.

It’s often justified by the need to update the UI, write assertions in tests, or provide immediate feedback to users. However, it’s mixing two fundamentally different concerns.

Why This is a Problem

This dual-purpose method blurs the boundary between doing something and asking about the state. In the example mentioned above, you can assume that we also have a GetBalance method that simply returns the current balance without changing the state. This method is intended to be used for querying the balance. As a result, we now have a duplication of logic. The same logic is spread across two very different places: Refund and GetBalance.

When Refund() both mutates state and returns the new balance, and GetBalance() also returns the balance, we now have two entry points into the logic of what the current balance is. If we later modify the way balance is calculated, maybe by including pending transactions , we now have two places to keep in sync. One is in a method that changes state, and the other in a supposedly safe query.

Even more importantly, the fact that Refund() also returns the balance muddies the water about its intent. Is it performing a refund? Is it reporting a balance? Both? That lack of clarity isn’t just theoretical it becomes a real maintenance hazard when others on the team reuse the method for the wrong reason (e.g., calling Refund() just to see what the updated balance is).

So yes, what seems like duplication is actually a signal of cross-cutting concerns bleeding into a single method. A cleaner design would reserve Refund() purely for state mutation, and keep GetBalance() focused entirely on retrieval, letting each method do one thing, and do it well.

When code mixes these responsibilities, it leads to several issues:

• The method is no longer predictable. You might expect it to simply return data, but it has side effects.
• It makes testing and reasoning harder. If your tests depend on calling Refund just to get a balance, they might unintentionally mutate the state.
• It introduces indeterminism. You can’t safely call the method without knowing whether it alters the system.
• It promotes duplication. If there’s a separate GetBalance() method that also returns Balance, you’re maintaining two ways of reaching the same logic, increasing coupling and cognitive load.

The fix is to separate responsibilities:

public void Refund(Money amount)
{
    Balance += amount;
}
public Money GetBalance()
{
    return Balance;
}

Isn’t That Overkill?

A common argument is: Now we need to call two methods instead of one. Isn’t that inefficient?

This is where use case orchestration comes into play.

Rather than merging logic inside a single method, delegate the orchestration to an Application Service or Use Case Handler, especially in domain-centric designs. For example:

public class WalletService
{
    public Money RefundAndGetBalance(long userId, Money amount)
    {
        var wallet = _walletRepository.GetByUserId(userId);
        wallet.Refund(amount);
        _walletRepository.Save(wallet);
        return wallet.GetBalance();
    }
}

Here, the coordination happens in one place , but each domain object or method still holds only a single responsibility.

But What About Performance?

Sometimes, mixed responsibility arises from a performance motivation. For instance, in database-heavy systems, minimizing roundtrips or expensive computations is a valid concern.

However, it’s worth noting that poorly structured code is rarely performant in the long run. In fact, messier code often leads to accidental inefficiencies that are hard to diagnose or fix later. Clean separation is not inherently slow , and you can still optimize with techniques like batching, caching, or introducing a facade that aggregates data without compromising domain purity.

Poorly structured code is rarely performant in the long run.

Real-World Scenario: Accounting Approval

Consider a case where an accounting manager is reviewing a financial document for approval. To make this decision, they need to see:

• Details of the accounting document
• The system that issued it (e.g., sales or procurement)
• The user who created it
• Summaries of related ledger accounts
• Pending approvals and statuses

All of this is query data, it supports decision making. Once the manager clicks Approve, the system executes a command that transitions the document state:

[HttpPost("/approve")]
public IActionResult ApproveDocument(long documentId)
{
    _documentService.Approve(documentId);
    return Ok();
}

[HttpGet("/document-summary/{id}")]
public IActionResult GetDocumentSummary(long id)
{
    var data = _documentQueryService.GetSummary(id);
    return Ok(data);
}

This flow reinforces the idea behind REST(Representation State Transfer). We change the state in one step (command) and retrieve its representation in another (query).

Domain vs Application Layer: Who Should Do What?

Another source of confusion comes from not clearly distinguishing the domain logic from the application logic.

Your domain model should focus on pure behavior: e.g., WithdrawFunds, MoveToNextState, etc. Application services or orchestrators handle things like loading data, sequencing calls, and coordinating commands and queries.

Let’s revisit a more complex orchestration:

public void FinalizePurchase(long orderId)
{
    var order = FetchOrder(orderId);
    
    if (wallet.Amount < order.TotalPrice)
        throw new Exception();

    wallet.Withdraw(order.TotalPrice);

    order.MarkAsPurchased();

    RecordFinancialTransaction(order);

    AddSupportNote(order, "Finalized successfully");

    NotifyUser(order.Buyer, "Your order is complete!");
}

This method clearly does multiple things. While it may be okay at the application layer, each internal method it calls should do one thing only. For example:

public void RecordFinancialTransaction(Order order)
{
    AddCredit(order.Id, order.TotalPrice);
    AddDebit(order.Id, order.TotalPrice);
}

This function too could be split or triggered asynchronously via events. After purchase is finalized you could publish an event:

public class OrderFinalizedEventHandler
{
    public void Handle(OrderFinalized e)
    {
        RecordFinancialTransaction(e.OrderId);
        LogSupportNote(e.OrderId);
    }
}

Special Case: Real-time UI Updates

One question that often comes up is: In scenarios where the state changes and we immediately need to show the updated value in the UI, is it okay to combine command and query?

The answer is still no, at least in principle.

Even in UI-driven or real-time systems, it’s usually better to keep state-changing operations and data retrieval separate. For example, you might first call an API to update something, and then make another API call (or await a subscription/event) to get the latest representation.

Especially in event-sourced architectures, the read model might lag behind due to eventual consistency. But this is usually a design trade-off — not a reason to revert to impure methods.

Moreover, read models often have different shapes, scales, and enrichment needs. They’re decision-support tools. The commands that result from those decisions are usually minimal and direct.

The Last Words

The Mixed Responsibility Smell is subtle, but widespread. It often arises from good intentions trying to simplify, optimize, or reduce code. But left unchecked, it leads to fragile and ambiguous codebases.

The key takeaway is this:

Every unit of code should have one clear reason to change.

If you find yourself writing a method that’s both changing state and returning information, pause and ask: Are these really part of the same concern? If not, extract and separate.

You’ll gain code that’s easier to reason about, test, extend, and even optimize in.