10.3: Refactoring Book. Refactoring for Command-Query Separation: Enhancing Java Patterns in Production Code

Sep 24, 2025

Brief introduction: this family of refactorings

What it is (short): Command–Query Separation (CQS) states that a routine should either perform an action (a command) or return data (a query), but not both at once. Refactoring “Separate Query from Modifier” is the practical transformation: find methods that return a value and change object state and split them so queries won’t cause side effects. (Wikipedia)

When to apply:

When a method that looks like a getter, validator or calculation also mutates object state (timestamps, caches, counters, DB writes, metrics).
When tests are brittle due to hidden state changes.
When side effects make code hard to compose, or you must call a function from assertions or logging and you must guarantee no state changes.
When moving toward clearer APIs (commands vs queries), or when preparing for architectural patterns such as CQRS. (martinfowler.com)

High-level practical benefits: easier reasoning (no hidden mutations), simpler unit tests, fewer race conditions in multi-threaded code, clearer APIs, and safer assertions/validators.

Example 1 — Lazy cache population that surprises callers

Scenario (real-world): A service exposes getUserPreferences(userId) which, if the cached preferences are missing, fetches from DB and writes back to cache. Caller expects a read; cache write is a surprising side effect (also may make tests slower/fragile and cause accidental DB writes during read-only flows).

Problematic code:

public class PreferencesService {
    private final Cache<String, Preferences> cache;
    private final PreferencesRepository repo;

    public PreferencesService(Cache<String, Preferences> cache, PreferencesRepository repo) {
        this.cache = cache;
        this.repo = repo;
    }

    // Query-looking method that mutates (writes) cache as a side-effect
    public Preferences getPreferences(String userId) {
        Preferences prefs = cache.get(userId);
        if (prefs == null) {
            prefs = repo.loadPreferences(userId);
            cache.put(userId, prefs); // <-- write side-effect hidden in “get”
        }
        return prefs;
    }
}

Why it’s a problem in practice:

Unit tests that call getPreferences for assertions also change cache state — order-dependent tests or false positives.
In read-only modes (e.g., admin read-only operations), calling this method can unexpectedly cause cache population or DB access patterns.
In distributed systems, cache writes may trigger eviction policies or metrics; hiding them inside a getter mixes concerns and makes auditing harder.

Refactored approaches (two variants):

A — Explicit separation: fetch (no side-effect) + populateCache (modifier)

public class PreferencesService {
    // Query: pure read (may still access repo but must not mutate cache)
    public Preferences fetchPreferences(String userId) {
        Preferences prefs = cache.get(userId);
        if (prefs != null) return prefs;
        // fallback load but explicitly return without mutating cache
        return repo.loadPreferences(userId);
    }

    // Command: explicit population of cache
    public void populateCache(String userId, Preferences prefs) {
        cache.put(userId, prefs);
    }
}

Usage pattern:

If you want to ensure cached data exists: Preferences p = service.fetchPreferences(id); service.populateCache(id, p); (or combine in a higher-level coordinator that decides when to write).
In read-only contexts, tests or code call only fetchPreferences to guarantee no cache mutation.

B — Return a FetchResult that carries value + intention, pushing mutation decision to caller

public final class FetchResult {
    private final Preferences prefs;
    private final boolean fromCache;

    public FetchResult(Preferences prefs, boolean fromCache) { ... }
    public Preferences prefs() { return prefs; }
    public boolean fromCache() { return fromCache; }
}

public FetchResult fetchPreferencesWithMeta(String userId) {
    Preferences p = cache.get(userId);
    if (p != null) return new FetchResult(p, true);
    return new FetchResult(repo.loadPreferences(userId), false);
}

Why either is better:

The side effect is explicit; callers can decide to cache or not (good for controllers, batch jobs, testing).
Tests and read-only components can call fetchPreferences* without any write side-effects.

Example 2 — Getter that updates last-access metadata (timestamp/counter)

Scenario: A Document object has getContent() that updates lastViewedAt and viewCount++ every time content is fetched. This causes surprising updates when logging, previewing, or running assertions.

Problematic code:

public class Document {
    private String content;
    private Instant lastViewedAt;
    private int viewCount;

    public String getContent() {
        this.lastViewedAt = Instant.now(); // hidden mutation
        this.viewCount++;
        return this.content;
    }
    // getters for lastViewedAt/viewCount omitted
}

Why it’s a problem in practice:

Display code that “just reads” an object to render a page causes analytics to change.
Tests asserting on view counts must stub or reset state before/after reads; intermittent failures appear.
If getContent() is used in an assertion or log that runs in a monitoring probe, probes change the production state — dangerous.

Refactor (single recommended approach): separate query and explicit mark/record)

public class Document {
    private final String content;
    private Instant lastViewedAt;
    private int viewCount;

    public Document(String content) { this.content = content; }

    // Query — pure read
    public String getContent() {
        return content;
    }

    // Command — explicit mark that user viewed the document
    public void markViewed(Instant when) {
        this.lastViewedAt = when;
        this.viewCount++;
    }

    // convenience: markViewedNow
    public void markViewedNow() {
        markViewed(Instant.now());
    }
}

Usage patterns and variants:

Controller route that handles a user viewing a document: String html = doc.getContent(); doc.markViewedNow();
If you want atomic semantics (read + record must be atomic), provide a coordinator method readAndMarkViewed()that plainly documents the mutation so callers consciously call it. This is allowed when semantics require atomicity, but the method name must make mutation explicit.

Why this is better: prevents accidental metric pollution, makes auditing obvious, and allows tests to call getContent()without changing analytics.

Example 3 — Method that both validates and persists (IO side-effects mixed with query)

Scenario: In many codebases you’ll find validateAndSave(entity) returning boolean success — it validates input and then persists to DB. Sometimes callers call it expecting just validation; sometimes they expect it to save. Mixing them makes flow control brittle and tight-couples validation and persistence.

Problematic code:

public class UserService {
    private final UserRepository repo;

    public boolean validateAndSave(User user) {
        if (!user.isValid()) return false;
        repo.save(user); // side-effect that persists
        return true;
    }
}

Why it’s a problem in practice:

Hard to reuse validation logic (e.g., for a “preview” or dry-run).
Tests for validation that call validateAndSave also write to DB (slower, needs rollbacks).
Hard to implement multi-step workflows (validate -> show confirmation -> save) without duplicating validation.

Refactored approaches (two complementary choices):

A — Pure validation + separate persistence

public ValidationResult validate(User user) {
    // run schema & business checks, return rich ValidationResult (errors, warnings)
}

public void save(User user) {
    repo.save(user); // command
}

B — Provide a “dry-run” flag or a higher-level operation that is explicit

public SaveOutcome saveIfValid(User user, boolean dryRun) {
    ValidationResult vr = validate(user);
    if (!vr.isValid()) return new SaveOutcome(false, vr);
    if (!dryRun) repo.save(user);
    return new SaveOutcome(true, vr);
}

Why this helps:

Tests can call validate() only (no side effects).
UI flows can run validate() to show errors, then call save() on explicit confirmation.
If you must offer a convenience combined operation, return a rich outcome object and name it clearly (saveIfValid, commitIfValid, validateAndCommit) so the mutation is explicit.

Example 4 — Collection operations that both mutate and return derived results

Scenario: Shopping carts, inventory adjustments, and batch processors often expose methods like removeItemAndGetRemaining() or allocateStockAndReturnAllocation() that mutate internal lists and also return derived values. This complicates composition and makes building transactional flows harder.

Problematic code:

public class Inventory {
    private final Map<String, Integer> stock = new HashMap<>();

    // removes item quantity and returns remaining count
    public int removeAndGetRemaining(String sku, int qty) {
        Integer current = stock.getOrDefault(sku, 0);
        if (current < qty) throw new IllegalArgumentException(”insufficient”);
        stock.put(sku, current - qty);
        return stock.get(sku); // returns mutated state
    }
}

Why it’s a problem in practice:

Callers that only want to know remaining stock may accidentally remove stock.
In concurrent/transactional flows, caller might want to compose checks and commits — mixing them forces awkward guard clauses.
Testing inventory check logic becomes destructive.

Refactored approaches (three variants):

A — Query + Command split

public int getStock(String sku) {
    return stock.getOrDefault(sku, 0);
}

public void remove(String sku, int qty) {
    int current = getStock(sku);
    if (current < qty) throw new IllegalArgumentException(”insufficient”);
    stock.put(sku, current - qty);
}

B — Use a Command object + separate result for intent

public final class Allocation {
    public final boolean success;
    public final int remaining;
    public Allocation(boolean success, int remaining) { ... }
}

public Allocation tryAllocate(String sku, int qty) {
    int current = getStock(sku);
    if (current < qty) return new Allocation(false, current);
    // Here we choose to mutate, but the name “tryAllocate” makes it explicit
    remove(sku, qty);
    return new Allocation(true, getStock(sku));
}

C — Two-phase approach for transactional needs (check -> commit)

public boolean canAllocate(String sku, int qty) { return getStock(sku) >= qty; }
public void commitAllocation(String sku, int qty) { remove(sku, qty); }

Why these work:

They give the caller the choice and clearly separate a check from a state change (important for transactions, and for tests that should run without mutating shared resources).

Example 5 — Lazy initialization + double-checked locking hidden in getter

Scenario: getHeavyResource() returns a resource but creates it on first use (lazily) and stores it in a field. If callers assume get is a simple accessor, they’ll be surprised by CPU/IO cost and possible race conditions if code is not thread-safe.

Problematic code:

public class HeavyHolder {
    private HeavyResource resource;

    public HeavyResource getResource() {
        if (resource == null) { // hidden expensive initialization
            resource = new HeavyResource(); // maybe slow, may throw, does IO
        }
        return resource;
    }
}

Why it’s a problem in practice:

Misleading API: callers may call getResource() in a tight loop expecting cheap operations but trigger expensive init.
In multi-threaded environments, the naive lazy-initialization has race conditions or double init.
Hard to test because initialization may open sockets, external files, or heavy CPU.

Refactored options (two safe approaches):

A — Explicit initialize + pure get

public void initialize() {
    synchronized(this) {
        if (resource == null) resource = new HeavyResource();
    }
}
public HeavyResource getResource() {
    if (resource == null) throw new IllegalStateException(”not initialized”);
    return resource;
}

B — Use Supplier/Optional + explicit computeIfAbsent semantics in constructor or factory

public static HeavyHolder createLazy(Supplier<HeavyResource> factory) {
    return new HeavyHolder(factory);
}
private final Supplier<HeavyResource> factory;
private volatile HeavyResource resource;

private HeavyHolder(Supplier<HeavyResource> factory) { this.factory = factory; }

public HeavyResource acquireResource() {
    if (resource == null) {
        synchronized(this) {
            if (resource == null) resource = factory.get();
        }
    }
    return resource;
}

Why this helps: The initialization intent is explicit, you avoid accidental heavy work in “getters” and can control threading semantics. If a get really must instantiate, name it acquireResource() or ensureResource() so mutation/creation intent is visible.

Example 6 — Using CQRS / read-model separation where queries are heavy or need different scaling

Scenario: In high-scale systems, reads are frequent and should not trigger writes. A pattern that evolves from CQS is CQRS (Command Query Responsibility Segregation) where read and write models are deliberately separated — queries don’t touch write-side aggregates or mutate state. This provides a different approach to the same problem at architectural level.

Problematic “mixed” approach (within a monolith):

// Single API that both updates projection and returns data
public UserProfile fetchAndUpdateProjection(UUID id) {
    User user = userRepo.findById(id);
    projectionService.updateLastRead(id); // mutation inside query
    return user.toProfile();
}

Why it’s a problem in practice:

Read latency increases because the query triggers writes.
Read errors could also cause writes to be partially applied, complicating consistency.
Scaling reads independently of writes becomes harder.

Refactored (CQRS-style): separate read endpoint and command endpoint; asynchronous projection updates

Read side (pure query):

public UserProfile getUserProfile(UUID id) {
    // reads from read-optimized store (maybe a denormalized view)
    return readStore.getProfile(id);
}

Command side (mutating):

public void recordProfileViewed(UUID id, UUID actor) {
    commandBus.send(new ProfileViewedCommand(id, actor));
}

Projection: Command handlers update a read-optimized store asynchronously (or synchronously, but via explicit command pipeline). The read API never mutates state.

Why this is beneficial: When you scale reads (cache layers, read replicas, specialized DB), read logic never alters state; commands go through their own pipeline. This is a natural extension of CQS into architecture (CQRS) for high-scale systems. (Medium)

Detailed reasoning for the refactorings (what issue each solves, context)

Hidden side-effects cause surprises and brittle tests. If a method named like a getter modifies state, tests that read state will change environment between assertions. Solution: split query and modifier so tests can call only the query. (Examples 1,2,3) (refactoring.guru)
Clearer API intent — Naming matters. Methods that mutate must be named to show the mutation (save, remove, markViewed, populateCache), while queries should be cheap and side-effect-free. This prevents the “principle of least surprise.”
Concurrency and race conditions. When query implicitly modifies state (e.g., lazy init), multi-threaded callers face races. Splitting lets you reason about atomicity (or purposely provide atomic combined methods with explicit names).
Composability and reusability. Queries can be called freely from logging, assertions, monitoring, and UIs without mutation; commands are used when you intentionally change state. This is useful in complex workflows (validate → confirm → commit). (Examples 3 & 4)
Testing and performance — Separating allows faster unit tests (no DB writes) and better-perf read paths (caches and read models) in production. (Examples 1 & 6)
Architectural scaling (CQRS). When reads vastly outnumber writes, separating read/write at architectural level (CQRS) enables independent scaling, caching and read-store optimization. (Example 6) (Medium)

Practical tips & patterns when applying this refactoring

Use intent-revealing names: getX(), isX() are queries. saveX(), removeX(), markX() are commands. If a method both reads and writes, give it a clearly mutating name (checkout(), popAndReturn()), or better: split it.
If atomicity is required, provide a clearly named combined method (readAndLock(), pop() for stack pop) — but document it and keep it minimal. Martin Fowler acknowledges pop() as an example where returning a value and mutating is common and reasonable — but that exception should be deliberate and documented. (martinfowler.com)
Return rich outcomes when helpful: ValidationResult, AllocationResult or Optional<T> with metadata help callers decide whether to call a modifier.
When performance matters, don’t hide lazy expensive work in thin getters — use ensureInitialized() or explicit initialization in startup.
When scaling reads, consider separate read models or caches and command pipelines (CQRS) to keep queries pure. (Kurrent - event-native data platform)

Conclusion (short & practical)

Separating queries from modifiers is a small, high-value refactoring that pays off in predictability, testability, and maintainability. It’s a local change with wide-reaching effects: fewer brittle tests, fewer accidental DB/cache writes, clearer APIs, and safer concurrency. For single-method smells, split the read and write paths and name them clearly; for systems with heavy read/write asymmetry, consider applying the pattern at a higher architectural level (CQRS).

If you’d like, I can:

Convert any one of the six examples into a copy/paste-ready Java module with unit tests (JUnit + Mockito) that demonstrate before/after test results.
Or produce an automated quick-check checklist (lint rules and code-review checklist) you can add to a PR template to detect mixed query/modifier methods.

Which would you prefer next?Brief introduction: this family of refactorings

When to apply:

When a method that looks like a getter, validator or calculation also mutates object state (timestamps, caches, counters, DB writes, metrics).
When tests are brittle due to hidden state changes.
When side effects make code hard to compose, or you must call a function from assertions or logging and you must guarantee no state changes.
When moving toward clearer APIs (commands vs queries), or when preparing for architectural patterns such as CQRS. (martinfowler.com)

High-level practical benefits: easier reasoning (no hidden mutations), simpler unit tests, fewer race conditions in multi-threaded code, clearer APIs, and safer assertions/validators.

Example 1 — Lazy cache population that surprises callers

Problematic code:

public class PreferencesService {
    private final Cache<String, Preferences> cache;
    private final PreferencesRepository repo;

    public PreferencesService(Cache<String, Preferences> cache, PreferencesRepository repo) {
        this.cache = cache;
        this.repo = repo;
    }

    // Query-looking method that mutates (writes) cache as a side-effect
    public Preferences getPreferences(String userId) {
        Preferences prefs = cache.get(userId);
        if (prefs == null) {
            prefs = repo.loadPreferences(userId);
            cache.put(userId, prefs); // <-- write side-effect hidden in “get”
        }
        return prefs;
    }
}

Why it’s a problem in practice:

Unit tests that call getPreferences for assertions also change cache state — order-dependent tests or false positives.
In read-only modes (e.g., admin read-only operations), calling this method can unexpectedly cause cache population or DB access patterns.
In distributed systems, cache writes may trigger eviction policies or metrics; hiding them inside a getter mixes concerns and makes auditing harder.

Refactored approaches (two variants):

A — Explicit separation: fetch (no side-effect) + populateCache (modifier)

public class PreferencesService {
    // Query: pure read (may still access repo but must not mutate cache)
    public Preferences fetchPreferences(String userId) {
        Preferences prefs = cache.get(userId);
        if (prefs != null) return prefs;
        // fallback load but explicitly return without mutating cache
        return repo.loadPreferences(userId);
    }

    // Command: explicit population of cache
    public void populateCache(String userId, Preferences prefs) {
        cache.put(userId, prefs);
    }
}

Usage pattern:

If you want to ensure cached data exists: Preferences p = service.fetchPreferences(id); service.populateCache(id, p); (or combine in a higher-level coordinator that decides when to write).
In read-only contexts, tests or code call only fetchPreferences to guarantee no cache mutation.

B — Return a FetchResult that carries value + intention, pushing mutation decision to caller

public final class FetchResult {
    private final Preferences prefs;
    private final boolean fromCache;

    public FetchResult(Preferences prefs, boolean fromCache) { ... }
    public Preferences prefs() { return prefs; }
    public boolean fromCache() { return fromCache; }
}

public FetchResult fetchPreferencesWithMeta(String userId) {
    Preferences p = cache.get(userId);
    if (p != null) return new FetchResult(p, true);
    return new FetchResult(repo.loadPreferences(userId), false);
}

Why either is better:

The side effect is explicit; callers can decide to cache or not (good for controllers, batch jobs, testing).
Tests and read-only components can call fetchPreferences* without any write side-effects.

Example 2 — Getter that updates last-access metadata (timestamp/counter)

Problematic code:

public class Document {
    private String content;
    private Instant lastViewedAt;
    private int viewCount;

    public String getContent() {
        this.lastViewedAt = Instant.now(); // hidden mutation
        this.viewCount++;
        return this.content;
    }
    // getters for lastViewedAt/viewCount omitted
}

Why it’s a problem in practice:

Display code that “just reads” an object to render a page causes analytics to change.
Tests asserting on view counts must stub or reset state before/after reads; intermittent failures appear.
If getContent() is used in an assertion or log that runs in a monitoring probe, probes change the production state — dangerous.

Refactor (single recommended approach): separate query and explicit mark/record)

public class Document {
    private final String content;
    private Instant lastViewedAt;
    private int viewCount;

    public Document(String content) { this.content = content; }

    // Query — pure read
    public String getContent() {
        return content;
    }

    // Command — explicit mark that user viewed the document
    public void markViewed(Instant when) {
        this.lastViewedAt = when;
        this.viewCount++;
    }

    // convenience: markViewedNow
    public void markViewedNow() {
        markViewed(Instant.now());
    }
}

Usage patterns and variants:

Controller route that handles a user viewing a document: String html = doc.getContent(); doc.markViewedNow();
If you want atomic semantics (read + record must be atomic), provide a coordinator method readAndMarkViewed()that plainly documents the mutation so callers consciously call it. This is allowed when semantics require atomicity, but the method name must make mutation explicit.

Why this is better: prevents accidental metric pollution, makes auditing obvious, and allows tests to call getContent()without changing analytics.

Example 3 — Method that both validates and persists (IO side-effects mixed with query)

Problematic code:

public class UserService {
    private final UserRepository repo;

    public boolean validateAndSave(User user) {
        if (!user.isValid()) return false;
        repo.save(user); // side-effect that persists
        return true;
    }
}

Why it’s a problem in practice:

Hard to reuse validation logic (e.g., for a “preview” or dry-run).
Tests for validation that call validateAndSave also write to DB (slower, needs rollbacks).
Hard to implement multi-step workflows (validate -> show confirmation -> save) without duplicating validation.

Refactored approaches (two complementary choices):

A — Pure validation + separate persistence

public ValidationResult validate(User user) {
    // run schema & business checks, return rich ValidationResult (errors, warnings)
}

public void save(User user) {
    repo.save(user); // command
}

B — Provide a “dry-run” flag or a higher-level operation that is explicit

public SaveOutcome saveIfValid(User user, boolean dryRun) {
    ValidationResult vr = validate(user);
    if (!vr.isValid()) return new SaveOutcome(false, vr);
    if (!dryRun) repo.save(user);
    return new SaveOutcome(true, vr);
}

Why this helps:

Tests can call validate() only (no side effects).
UI flows can run validate() to show errors, then call save() on explicit confirmation.
If you must offer a convenience combined operation, return a rich outcome object and name it clearly (saveIfValid, commitIfValid, validateAndCommit) so the mutation is explicit.

Example 4 — Collection operations that both mutate and return derived results

Problematic code:

public class Inventory {
    private final Map<String, Integer> stock = new HashMap<>();

    // removes item quantity and returns remaining count
    public int removeAndGetRemaining(String sku, int qty) {
        Integer current = stock.getOrDefault(sku, 0);
        if (current < qty) throw new IllegalArgumentException(”insufficient”);
        stock.put(sku, current - qty);
        return stock.get(sku); // returns mutated state
    }
}

Why it’s a problem in practice:

Callers that only want to know remaining stock may accidentally remove stock.
In concurrent/transactional flows, caller might want to compose checks and commits — mixing them forces awkward guard clauses.
Testing inventory check logic becomes destructive.

Refactored approaches (three variants):

A — Query + Command split

public int getStock(String sku) {
    return stock.getOrDefault(sku, 0);
}

public void remove(String sku, int qty) {
    int current = getStock(sku);
    if (current < qty) throw new IllegalArgumentException(”insufficient”);
    stock.put(sku, current - qty);
}

B — Use a Command object + separate result for intent

public final class Allocation {
    public final boolean success;
    public final int remaining;
    public Allocation(boolean success, int remaining) { ... }
}

public Allocation tryAllocate(String sku, int qty) {
    int current = getStock(sku);
    if (current < qty) return new Allocation(false, current);
    // Here we choose to mutate, but the name “tryAllocate” makes it explicit
    remove(sku, qty);
    return new Allocation(true, getStock(sku));
}

C — Two-phase approach for transactional needs (check -> commit)

public boolean canAllocate(String sku, int qty) { return getStock(sku) >= qty; }
public void commitAllocation(String sku, int qty) { remove(sku, qty); }

Why these work:

They give the caller the choice and clearly separate a check from a state change (important for transactions, and for tests that should run without mutating shared resources).

Example 5 — Lazy initialization + double-checked locking hidden in getter

Problematic code:

public class HeavyHolder {
    private HeavyResource resource;

    public HeavyResource getResource() {
        if (resource == null) { // hidden expensive initialization
            resource = new HeavyResource(); // maybe slow, may throw, does IO
        }
        return resource;
    }
}

Why it’s a problem in practice:

Misleading API: callers may call getResource() in a tight loop expecting cheap operations but trigger expensive init.
In multi-threaded environments, the naive lazy-initialization has race conditions or double init.
Hard to test because initialization may open sockets, external files, or heavy CPU.

Refactored options (two safe approaches):

A — Explicit initialize + pure get

public void initialize() {
    synchronized(this) {
        if (resource == null) resource = new HeavyResource();
    }
}
public HeavyResource getResource() {
    if (resource == null) throw new IllegalStateException(”not initialized”);
    return resource;
}

B — Use Supplier/Optional + explicit computeIfAbsent semantics in constructor or factory

public static HeavyHolder createLazy(Supplier<HeavyResource> factory) {
    return new HeavyHolder(factory);
}
private final Supplier<HeavyResource> factory;
private volatile HeavyResource resource;

private HeavyHolder(Supplier<HeavyResource> factory) { this.factory = factory; }

public HeavyResource acquireResource() {
    if (resource == null) {
        synchronized(this) {
            if (resource == null) resource = factory.get();
        }
    }
    return resource;
}

Example 6 — Using CQRS / read-model separation where queries are heavy or need different scaling

Problematic “mixed” approach (within a monolith):

// Single API that both updates projection and returns data
public UserProfile fetchAndUpdateProjection(UUID id) {
    User user = userRepo.findById(id);
    projectionService.updateLastRead(id); // mutation inside query
    return user.toProfile();
}

Why it’s a problem in practice:

Read latency increases because the query triggers writes.
Read errors could also cause writes to be partially applied, complicating consistency.
Scaling reads independently of writes becomes harder.

Refactored (CQRS-style): separate read endpoint and command endpoint; asynchronous projection updates

Read side (pure query):

public UserProfile getUserProfile(UUID id) {
    // reads from read-optimized store (maybe a denormalized view)
    return readStore.getProfile(id);
}

Command side (mutating):

public void recordProfileViewed(UUID id, UUID actor) {
    commandBus.send(new ProfileViewedCommand(id, actor));
}

Projection: Command handlers update a read-optimized store asynchronously (or synchronously, but via explicit command pipeline). The read API never mutates state.

Detailed reasoning for the refactorings (what issue each solves, context)

Hidden side-effects cause surprises and brittle tests. If a method named like a getter modifies state, tests that read state will change environment between assertions. Solution: split query and modifier so tests can call only the query. (Examples 1,2,3) (refactoring.guru)
Clearer API intent — Naming matters. Methods that mutate must be named to show the mutation (save, remove, markViewed, populateCache), while queries should be cheap and side-effect-free. This prevents the “principle of least surprise.”
Concurrency and race conditions. When query implicitly modifies state (e.g., lazy init), multi-threaded callers face races. Splitting lets you reason about atomicity (or purposely provide atomic combined methods with explicit names).
Composability and reusability. Queries can be called freely from logging, assertions, monitoring, and UIs without mutation; commands are used when you intentionally change state. This is useful in complex workflows (validate → confirm → commit). (Examples 3 & 4)
Testing and performance — Separating allows faster unit tests (no DB writes) and better-perf read paths (caches and read models) in production. (Examples 1 & 6)
Architectural scaling (CQRS). When reads vastly outnumber writes, separating read/write at architectural level (CQRS) enables independent scaling, caching and read-store optimization. (Example 6) (Medium)

Practical tips & patterns when applying this refactoring

Use intent-revealing names: getX(), isX() are queries. saveX(), removeX(), markX() are commands. If a method both reads and writes, give it a clearly mutating name (checkout(), popAndReturn()), or better: split it.
If atomicity is required, provide a clearly named combined method (readAndLock(), pop() for stack pop) — but document it and keep it minimal. Martin Fowler acknowledges pop() as an example where returning a value and mutating is common and reasonable — but that exception should be deliberate and documented. (martinfowler.com)
Return rich outcomes when helpful: ValidationResult, AllocationResult or Optional<T> with metadata help callers decide whether to call a modifier.
When performance matters, don’t hide lazy expensive work in thin getters — use ensureInitialized() or explicit initialization in startup.
When scaling reads, consider separate read models or caches and command pipelines (CQRS) to keep queries pure. (Kurrent - event-native data platform)

Conclusion (short & practical)

Ivan’s Substack

Discussion about this post

Ready for more?