Engineering

Two feature modules that needed each other, and the type that fixed it

A direct dependency between siblings is a cycle waiting for the third one.

MobileByteSensei 10 min read
kmparchitecturemodularity

TL;DR

  • feature/cart needed something from feature/checkout and vice versa. The build refused the second edge — Gradle will not resolve a circular project dependency — and the workaround was a shared type dumped in core/common, which grew to 61 unrelated classes over a year.
  • A sibling dependency is not wrong because of a rule. It is wrong because the shared concept has not been named, and the cycle is the build telling you so.
  • Naming it correctly took three questions and produced four small modules rather than one dumping ground, with the 61 classes redistributed and 9 deleted.
  • What reading on buys: the three questions, why core/common is an anti-pattern with a measurable cost, and the dependency check that stops the next one.

The case this post is about

Two features that genuinely needed each other:

// feature/checkout — needs to know what is in the cart
import com.example.feature.cart.CartSummary
class CheckoutViewModel(private val cart: CartRepository) { … }
// feature/cart — needs to know whether checkout is available for this cart
import com.example.feature.checkout.CheckoutEligibility
class CartViewModel(private val eligibility: CheckoutEligibility) { … }
// feature/cart/build.gradle.kts
implementation(projects.feature.checkout)
// feature/checkout/build.gradle.kts
implementation(projects.feature.cart)
$ ./gradlew build
FAILURE: Circular dependency between the following tasks:
  :feature:cart:compileKotlinJvm
  \--- :feature:checkout:compileKotlinJvm
       \--- :feature:cart:compileKotlinJvm (*)

The symptom

Not the cycle — that failed immediately and visibly, which is the build working. The symptom is what the team did next, and what it cost.

The workaround was a module for things two features share:

core/common/
├── CartSummary.kt
├── CheckoutEligibility.kt
└── … 59 more files, added over 14 months
# core/common, measured
  classes                                    61
  modules depending on it                    17 of 17
  classes used by more than one module       12
  classes used by exactly one module         40
  classes used by NOTHING                     9

Forty of sixty-one were used by one module — so they were not shared, they were relocated. Nine were used by nothing. And because every module depended on core/common, a change to any of the 61 recompiled all seventeen:

# touch one file in core/common
$ ./gradlew assemble --dry-run | grep -c compileKotlin
94

Ninety-four compile tasks for a one-line change to a class with one consumer.

A shared-things module measured by consumers per class, with nine classes used by nothing and forty used by exactly one module, so two thirds of it was relocated rather than shared

Why the obvious fix failed

The first fix was to make the dependency one-directional, which is the right instinct and needs a decision nobody had made:

// feature/cart → feature/checkout only
implementation(projects.feature.checkout)

Which compiles, and makes cart depend on checkout — so opening the cart pulls in the whole checkout feature, and a change to checkout recompiles cart. It also encodes a claim about the domain that is false: a cart does not require a checkout to exist.

The second fix was an interface in one feature implemented by the other, which is a real pattern and put the seam in the wrong place:

// feature/cart
interface EligibilityChecker { suspend fun isEligible(cart: CartSummary): Boolean }
// feature/checkout
class CheckoutEligibilityChecker : EligibilityChecker      // implements cart's interface

checkout now depends on cart, one edge, no cycle. And cart owns an abstraction about checkout, which means a change to checkout's eligibility rules changes an interface in cart. The dependency is inverted and the coupling is not reduced.

The third fix — the one that shipped for fourteen months — was core/common, which works and defers the decision indefinitely. The cost is the 94 compile tasks and the 40 relocated classes, and neither is visible until somebody counts.

The actual mechanism

A cycle between two modules means a concept exists that belongs to neither, and the build is refusing to let you avoid naming it.

CartSummary is not a cart concept. It is an order's contents: a list of line items with quantities and a total. The cart presents it, checkout prices it, order history displays it, and the receipt renders it. Four consumers, no owner among them — so it belongs below all four.

CheckoutEligibility is not a checkout concept either. It is a predicate over an order's contents and a customer's state. Checkout enforces it, the cart previews it, and a promotional banner reads it.

So the cycle is diagnostic. Two features needing each other is the build's way of saying that the thing passing between them has no home — and the fix is to give it one, below both, which removes both edges rather than reversing one.

And core/common is the specific failure of doing this without naming anything. A module whose name describes who uses it rather than what it contains has no admission criterion, so everything that is awkward to place goes there. The measurable consequence is that it becomes a universal dependency, and a universal dependency is a universal recompile.

The three questions that produced the right split:

  1. What is this concept, in the domain's own words?    "the contents of an order"
     — not "the thing cart and checkout share"
  2. Who ARE its consumers, listed?                      cart, checkout, orders, receipt
     — if the answer is one module, it belongs IN that module
  3. Does it depend on anything above it?                no — it is data plus pure predicates
     — if yes, it is not below both, and the split is wrong

Question 2 is the one that eliminated 40 of the 61 classes: a class with one consumer is not shared, and moving it back is free.

Which modules rebuild when checkout rules change under each arrangement, showing that inverting the dependency moves the arrow and rebuilds the same set while moving the concept down changes it

The fix

Four named modules below the features, with the 61 classes redistributed by their actual consumers.

core/
├── order-model/          what an order IS — data, no behaviour
│   ├── OrderContents.kt      (was CartSummary — renamed to what it is)
│   ├── LineItem.kt
│   └── Money.kt
├── order-rules/          pure predicates OVER order-model. Depends on it, nothing else.
│   ├── Eligibility.kt        (was CheckoutEligibility)
│   └── DiscountRules.kt
├── order-data/           persistence and network for orders. Depends on the two above.
│   └── OrderRepository.kt
└── (core/common — DELETED)
// core/order-model/build.gradle.kts
//
// No dependencies on anything except the standard library and serialization. That is the
// admission criterion this module HAS and core/common did not: if a class needs something
// from a layer above, it does not belong here, and the build enforces it.
kotlin {
    sourceSets.commonMain.dependencies {
        implementation(libs.kotlinx.serialization.json)
        // deliberately nothing else — see the gate below
    }
}
// build-logic/src/main/kotlin/layer-boundaries.gradle.kts
//
// The admission criterion, as a dependency rule. `core/common` grew to 61 classes because
// nothing could say what did not belong in it — a module named after its consumers has no
// criterion, so the criterion has to be the dependency set.
val LAYERS = listOf(
    "core:order-model" to emptySet<String>(),
    "core:order-rules" to setOf("core:order-model"),
    "core:order-data"  to setOf("core:order-model", "core:order-rules"),
)

afterEvaluate {
    val layer = LAYERS.firstOrNull { project.path == ":${it.first}" } ?: return@afterEvaluate
    val (name, allowed) = layer

    val actual = configurations
        .flatMap { it.dependencies.filterIsInstance<ProjectDependency>() }
        .map { it.dependencyProject.path.removePrefix(":") }
        .toSet()

    val violations = actual - allowed
    if (violations.isNotEmpty()) {
        error(
            "$name depends on ${violations.joinToString()}, which is above it.\n" +
            "  This module's allowed dependencies are: ${allowed.ifEmpty { setOf("(none)") }}.\n" +
            "  A type that needs something from a higher layer does not belong in this module —\n" +
            "  which is the criterion core/common lacked, and why it reached 61 classes."
        )
    }

    // A module nothing depends on is not shared infrastructure; it is a module. Reported
    // rather than failed, because a brand-new layer legitimately has no consumers for a day.
    val consumers = rootProject.subprojects.count { p ->
        p.configurations.any { c ->
            c.dependencies.filterIsInstance<ProjectDependency>().any { it.dependencyProject == project }
        }
    }
    if (consumers == 1) {
        logger.warn("$name has exactly ONE consumer. A type with one consumer belongs IN that " +
                    "module — 40 of core/common's 61 classes were in this state.")
    }
}

And the output it produced, after the redistribution:

$ ./gradlew build
BUILD SUCCESSFUL        # no cycle: both features depend DOWN, neither on the other

$ # the 61 classes, redistributed
  core/order-model          12   (genuinely multi-consumer data)
  core/order-rules           7   (pure predicates over it)
  core/order-data            5
  moved back into one feature   28   (one consumer each)
  deleted                        9   (no consumers)

$ # and the recompile cost of a one-line change
$ ./gradlew assemble --dry-run | grep -c compileKotlin
  touch core/common/CartSummary.kt (before)      94
  touch core/order-model/OrderContents.kt        31
  touch feature/cart/CartFilter.kt (moved back)   6

How we proved it

That 40 of 61 were not shared — counted from the import graph rather than judged:

  consumers per class, core/common
    0 consumers      9   ← dead
    1 consumer      40   ← relocated, not shared
    2 consumers      7
    3+ consumers     5

Two thirds of the module had a single consumer. That is not a shared module, it is a module where things were put.

That a universal dependency is a universal recompile. Measured, because this is the cost nobody attributes to the workaround:

  file touched                              compile tasks in the graph
  core/common (any of the 61)                        94
  core/order-model                                   31
  feature/cart, after the move back                   6

A one-line change to a one-consumer class recompiled ninety-four tasks for fourteen months.

That the interface-inversion fix did not reduce coupling. The measurement is which modules rebuild when checkout's rules change, which is the thing coupling means:

  arrangement                          modules rebuilt by a checkout rule change
  cycle (did not build)                   —
  cart → checkout                         cart, checkout, + cart's 4 consumers
  interface owned by cart                 cart, checkout, + cart's 4 consumers
  both → core/order-rules                 checkout, order-rules' 3 consumers

Inverting the dependency moved the arrow and rebuilt the same set. Only moving the concept down changed it.

That the one-consumer warning fires on real cases. Run across the whole tree after the split, because the rule is only useful if it finds things:

$ ./gradlew build 2>&1 | grep 'ONE consumer'
  core:order-data has exactly ONE consumer. …

order-data has one consumer today, which is legitimate — it was split for the dependency rule rather than for sharing — and the warning is correct to say so. We left it, with a comment, which is what a warning rather than a failure is for.

That the layer gate catches the regression it exists for. The fixture is the first thing somebody will try when order-model needs a repository:

$ ./gradlew :fixtures:order-model-violation:build
FAILURE: core:order-model depends on core:order-data, which is above it.
  This module's allowed dependencies are: (none).
  A type that needs something from a higher layer does not belong in this module —
  which is the criterion core/common lacked, and why it reached 61 classes.

What I cannot tell you is whether four modules is the right number. It is the number the three questions produced for this domain, and order-data having one consumer suggests it might be three. The split cost a day and the 94-to-31 recompile improvement is real; whether a different cut would have done better is not something I measured, because I only did it once.

And an honest residual. Nine deleted classes were dead for an unknown length of time — the git history says the most recent was last referenced eleven months ago. Nothing had been reporting them, and the one-consumer warning does not catch zero-consumer classes within a module, only zero-consumer modules. That gap is still open.

The sixty-one classes redistributed by actual consumer and the recompile cost of a one-line change falling from ninety-four compile tasks to six for a class that was moved back into its single consumer

What this generalizes to

A dependency cycle is usually not a layering mistake to be routed around but a missing name: two components need each other because something that belongs to neither is passing between them, and the build is refusing to let the question go unanswered. The tempting answer — a module for shared things — defers it at a cost that compounds, because a module named after its consumers has no admission criterion and therefore no reason to refuse anything. Name the concept in the domain's words, list its consumers, and if the list has one entry it was never shared.


Keep reading

One post a week on shipping mobile software. No digests, no course pitch. Subscribe.

One post a week, no digest

New writing when it lands. No roundups, no course pitch, no "10 tips".

Follow by RSS