Change Preventers — Find the Bug¶
12 buggy snippets where Change Preventers hide a bug. Diagnose; the cure is structural.
Bug 1 — Forgotten field on shotgun edit (Java)¶
class Customer {
private String name;
private String email;
private String phone; // newly added
}
class CustomerDto {
public String name;
public String email;
// forgot to add phone
}
class CustomerMapper {
public CustomerDto toDto(Customer c) {
CustomerDto dto = new CustomerDto();
dto.name = c.getName();
dto.email = c.getEmail();
return dto;
}
}
Diagnosis
A field added to `Customer` but missed in `CustomerDto` and `CustomerMapper`. The API silently returns no phone. Tests that don't check `phone` pass. **Why it hid:** Shotgun Surgery — adding a field requires editing 3 files; missing one is invisible until someone reports the bug. **Fix:** MapStruct (or any code-gen mapper). The mapper is regenerated when fields change, breaking compile if mappings can't be inferred.Bug 2 — Divergent Change introduces conflict (Java)¶
class UserManager {
public void updateProfile(User u, String newEmail) {
u.setEmail(newEmail);
userRepo.save(u);
}
public void changeSubscription(User u, Plan plan) {
u.setSubscriptionPlan(plan);
u.setSubscriptionRenewalDate(LocalDate.now().plusMonths(1));
userRepo.save(u);
}
// ... many more methods
}
Two engineers commit on the same day: - Engineer A modifies updateProfile to send a confirmation email. - Engineer B modifies changeSubscription to also update the analytics database.
Both PRs touch UserManager.java. Both get merged. Either:
(a) merge conflict (visible) — they resolve it together;
(b) auto-merge silently combines, but Engineer B's changes inadvertently affect imports/structure that break Engineer A's email logic.
Diagnosis
Divergent Change: one class is touched by everyone for everything. Cure: split UserManager into focused services. With per-service files, the two engineers wouldn't conflict. This isn't a bug in any single line — it's a *process* bug induced by structural smell.Bug 3 — Mapper drift (Python)¶
@dataclass
class User:
id: str
name: str
email: str
is_admin: bool
salary: int # added 2 weeks ago
# Hand-rolled DTO converter — never updated
def user_to_dto(user):
return {
"id": user.id,
"name": user.name,
"email": user.email,
"is_admin": user.is_admin,
# salary missing!
}
Caller:
Diagnosis
Hand-rolled mapper drift. Engineer added `salary` to `User`; nobody remembered to update `user_to_dto`. Python doesn't catch this at compile. **Fix:** use Pydantic or `dataclasses.asdict`: Or Pydantic with explicit projection (when you want to *exclude* fields):Bug 4 — Parallel hierarchy missing sibling (Java)¶
abstract class PaymentMethod { ... }
class CreditCard extends PaymentMethod { ... }
class Paypal extends PaymentMethod { ... }
class BankTransfer extends PaymentMethod { ... }
abstract class PaymentValidator {
public abstract boolean validate(PaymentMethod m);
}
class CreditCardValidator extends PaymentValidator { ... }
class PaypalValidator extends PaymentValidator { ... }
// no BankTransferValidator
class PaymentService {
public boolean validate(PaymentMethod m) {
if (m instanceof CreditCard) return new CreditCardValidator().validate(m);
if (m instanceof Paypal) return new PaypalValidator().validate(m);
if (m instanceof BankTransfer) return true; // ?!
return false;
}
}
Diagnosis
`BankTransferValidator` was forgotten. The author punted with `return true` — bank transfers skip validation entirely. Real money flows through unverified. (Or worse: silently fails — `return false` blocks legitimate transfers.) **Why it hid:** Parallel Inheritance Hierarchies. Adding a payment kind requires adding a validator; nobody enforces this. **Fix:** put validation on `PaymentMethod`. Now the parallel hierarchy is gone. Adding a payment method without validation is a compile error.Bug 5 — Cross-cutting concern not applied (Python)¶
@trace
def fetch_user(id): ...
@trace
def fetch_order(id): ...
# 50 more functions, all decorated
def fetch_payment(id): # forgot @trace
return db.query("SELECT * FROM payments WHERE id=?", id)
When debugging a slow request, the trace shows fetch_user and fetch_order timings but nothing for fetch_payment — making it look like fetch_payment is fast or absent.
Diagnosis
Cross-cutting concern (tracing) applied per-function. One missed → invisible call. This is Shotgun Surgery for tracing. **Fix:** apply at coarser granularity — module-level autoinstrumentation, or middleware that wraps the entire request: Now every request is traced as a whole; sub-spans for specific operations are added on demand. No per-function decoration needed.Bug 6 — Divergent Change merge bug (Go)¶
package user
type Service struct {
db *sql.DB
}
// Engineer A's commit:
func (s *Service) UpdateEmail(id int, email string) error {
_, err := s.db.Exec("UPDATE users SET email = ? WHERE id = ?", email, id)
return err
}
// Engineer B's commit (lands later, both touched same package):
func (s *Service) UpdatePassword(id int, hash string) error {
_, err := s.db.Exec("UPDATE users SET password_hash = ? WHERE id = ?", hash, id)
audit.Log(id, "password_changed") // new dependency
return err
}
Both compile in isolation. Both merge cleanly. But the test file:
package user_test
func TestUpdateEmail(t *testing.T) {
s := user.NewService(testDB) // doesn't initialize audit
s.UpdateEmail(1, "new@example.com")
// ...
}
A's test was fine. B's PR added audit dependency. After merge, A's test crashes (audit is nil) — even though A didn't change anything.
Diagnosis
Divergent Change at the package level. Two engineers' independent changes interact via shared dependencies. The fact that they touched the *same package* is the smell — they should have been working in different files / packages. **Fix:** split `user.Service` into `user.IdentityService` (UpdateEmail) and `user.AuthService` (UpdatePassword). Each service has its own constructor and dependencies. Tests can construct one without needing the other.Bug 7 — Schema drift across services (Multi-language)¶
// shared.proto (the canonical schema)
message Order {
string id = 1;
repeated string item_ids = 2;
int64 total_cents = 3;
string status = 4; // newly added
}
// Service A (compiled with v1 of proto)
Order order = Order.newBuilder().setId(...).setTotalCents(100).build();
// status not set — defaults to ""
sender.send(order);
# Service B (compiled with v2 of proto, expects status)
def handle(order):
if order.status == "PAID":
process_payment(order)
# status == "" -> no payment processing
Diagnosis
Service A wasn't redeployed after the proto changed. Its messages have empty `status`. Service B silently skips them. This is Shotgun Surgery at the architectural level — schema changes require coordinated redeployment. **Fix:** 1. Schema-versioning policy: only additive changes (new optional fields); no required-field additions. 2. Service B handles "missing status" gracefully (treat as legacy). 3. Build pipeline ensures shared `.proto` versions are bumped together. For breaking changes, use a separate message type and migrate consumers explicitly.Bug 8 — Forgotten validator in parallel hierarchy (Python)¶
class Notification(ABC):
@abstractmethod
def send(self): ...
class EmailNotification(Notification): ...
class SmsNotification(Notification): ...
class PushNotification(Notification): ...
# Parallel hierarchy of validators (Pydantic-like)
class EmailValidator: ...
class SmsValidator: ...
# No PushValidator — was added with PushNotification but validator forgotten
# Caller:
def validate_and_send(notification):
validators = {
"email": EmailValidator,
"sms": SmsValidator,
# forgot push
}
validator_class = validators.get(notification.channel)
if validator_class:
validator_class().validate(notification)
notification.send()
Diagnosis
`PushNotification` is sent without validation — `validators.get("push")` returns None, the conditional skips. The comment "forgot push" is the root cause. **Fix:** put validation on the notification itself. Adding a notification kind without validation is a runtime error at instantiation.Bug 9 — Divergent Change in test fixtures (Java)¶
class TestFixtures {
public static User testUser() {
User u = new User();
u.setName("Test");
u.setEmail("test@example.com");
return u;
}
public static Order testOrder() {
Order o = new Order();
o.setId("order-1");
o.setUser(testUser());
return o;
}
public static Payment testPayment() {
Payment p = new Payment();
p.setUser(testUser());
return p;
}
// ... 30 more `testXxx` methods touching multiple aggregates
}
Engineer A modifies testUser to add a required field. Engineer B's tests using testOrder (which calls testUser) start failing with no obvious reason.
Diagnosis
`TestFixtures` is a god class for test data. Modifying one fixture cascades. Divergent Change at test-data level. **Fix:** focused builders per concept. Each builder is scoped to one type. Changes to user fixture don't break unrelated tests.Bug 10 — Refused Bequest in inheritance (Java)¶
abstract class Animal {
public abstract void feed();
public abstract void exercise();
}
class Dog extends Animal {
public void feed() { ... }
public void exercise() { ... }
}
class Goldfish extends Animal {
public void feed() { ... }
public void exercise() {
throw new UnsupportedOperationException("Goldfish don't exercise");
}
}
// Caller:
List<Animal> pets = ...;
for (Animal a : pets) {
a.feed();
a.exercise(); // crashes on goldfish
}
Diagnosis
Refused Bequest masquerading as Parallel Inheritance — but the smell is also a Change Preventer (adding new pet types requires deciding "do they exercise?" implicitly). **Fix:** Capability-based design.interface Feedable { void feed(); }
interface Exerciseable { void exercise(); }
class Dog implements Feedable, Exerciseable { ... }
class Goldfish implements Feedable { ... } // doesn't claim Exerciseable
// Caller:
for (Feedable a : pets) a.feed();
for (Exerciseable a : pets) {
if (a instanceof Exerciseable e) e.exercise();
}
// Better — split lists:
List<Feedable> needFeeding = ...;
List<Exerciseable> needExercise = ...;
Bug 11 — Migration script touches god class (Python)¶
# migration_2024_03_add_country.py
def migrate(db):
# Add country to users
db.execute("ALTER TABLE users ADD COLUMN country VARCHAR(2) DEFAULT 'US'")
# Update User model dataclass... wait, that's in the application code
# Update User Pydantic schema... also application code
# Update GraphQL schema... also application
# ... migration script tries to coordinate all of these
Diagnosis
The migration script can't realistically modify application code. The "right" approach is a coordinated PR that includes: 1. Migration SQL. 2. Updated `User` model. 3. Updated DTO. 4. Updated GraphQL schema. 5. Updated test fixtures. This PR has Shotgun Surgery built-in — 5+ files touched together. Reviewers struggle to verify completeness. **Fix:** combine code generation (one source of truth → all derived types) + a database migration that's the single new addition. The PR shrinks to 2 files (the proto/Pydantic schema + the SQL migration). Some teams achieve this with **schema migrations driving everything** — the schema migration tool generates the model code. Others use schema-first design with code generation.Bug 12 — Distributed monolith deployment ordering (Multi-service)¶
A feature requires:
- Service A version 2.5 (publishes new event format)
- Service B version 3.1 (consumes new event format)
If A is deployed first, B is still on the old format → events fail to deserialize. If B is deployed first, A still publishes old format → B blocked waiting for events.
Diagnosis
Distributed monolith — the architectural Shotgun Surgery. Two services must deploy in coordinated order. **Fix patterns:** 1. **Backwards/forwards compatibility in the schema.** A's v2.5 publishes both old and new formats; B's v3.1 reads both. Once both are deployed, drop the old format in a follow-up release. 2. **Feature flags.** Both services know the new feature; deployments happen freely; the feature is enabled via flag once both are deployed. 3. **Single repo with single artifact** if the services genuinely cannot be decoupled — recognize that "two services" is fiction; the deployment unit is one. The architectural smell can't be cured by a single-PR refactor. It requires either schema discipline (compatibility) or honest service boundaries.Next: optimize.md — inefficient Change Preventer cures.