From code-foundations
Refactors deep nesting (3+ levels), complex conditionals, loops, high cyclomatic complexity (>10), and callback hell using guard clauses, intermediate variables, and loop best practices. Targets arrow-shaped code, repeated conditions.
npx claudepluginhub ryanthedev/code-foundationsThis skill uses the workspace's default tool permissions.
- Check nesting depth before adding levels (max 3)
Analyzes PHP code for nesting depth issues, detecting deep nesting over 3 levels, complex conditionals, nested loops, and callback hell. Suggests early returns and method extractions for refactoring.
Enforces Laravel complexity guardrails: cyclomatic ≤7, ≤80 lines per function, guard clauses, helper extraction, table-driven/strategy patterns over switches. Flags mixed concerns and hard-to-test paths.
Detects and simplifies overly complex code using KISS principle. Flattens deep nesting (>3 levels), extracts long functions (>30 lines), reduces cyclomatic complexity (≤10), and applies simplification checklist.
Share bugs, ideas, or general feedback.
| Threshold | Value | Source |
|---|---|---|
| Max nesting depth | 3 levels | Chomsky, Weinberg via Yourdon 1986a |
| McCabe complexity | >10 decision points = redesign | McCabe 1976 |
| Else clause consideration | 50-80% of ifs need else | Elshoff 1976 |
| Loop-with-exit comprehension | 25% better than top/bottom test | Soloway 1983 |
| Mental entity limit | 5-9 items | Miller 1956 |
Key Principles:
true/false not 0/1 for booleansfor when iteration count is known, while when unknown, foreach for collectionsif, error case in else (exception: guard clauses invert this intentionally—see below)NEVER Skip (even under time pressure):
Reconciling with "nominal in if": Guard clauses are an exception pattern where error/invalid cases exit early at the top of a function. This contradicts the general "nominal in if" rule intentionally—the goal is to get preconditions out of the way so the nominal path flows without nesting. Use guard clauses at function entry; use "nominal in if" for conditionals within the function body.
// BEFORE: Arrow-shaped code (4 levels) [ANTI-PATTERN]
if (file.validName()) {
if (file.open()) {
if (encryptionKey.valid()) {
if (file.decrypt(encryptionKey)) {
// lots of code (nominal case buried)
}
}
}
}
// AFTER: Guard clauses flatten structure
if (!file.validName()) return;
if (!file.open()) return;
if (!encryptionKey.valid()) return;
if (!file.decrypt(encryptionKey)) return;
// lots of code (nominal case at top level)
// BEFORE: Monstrous test [ANTI-PATTERN]
if (inputStatus == SUCCESS && printerReady && !printerBusy &&
paperAvailable && paperLevel > MIN_PAPER &&
documentValid && !documentEmpty && documentSize < MAX_SIZE) {
print();
}
// AFTER: Named intermediate booleans
bool printerCanPrint = printerReady && !printerBusy &&
paperAvailable && paperLevel > MIN_PAPER;
bool documentCanBePrinted = documentValid && !documentEmpty &&
documentSize < MAX_SIZE;
if (inputStatus == SUCCESS && printerCanPrint && documentCanBePrinted) {
print();
}
// BEFORE: Impossible to verify correct array access [ANTI-PATTERN]
for (int i = 0; i < numPayCodes; i++) {
for (int j = 0; j < 12; j++) {
for (int k = 0; k < numDivisions; k++) {
sum = sum + transaction[j][i][k]; // Is this right? Who knows!
}
}
}
// AFTER: Self-documenting and verifiable
for (int payCodeIdx = 0; payCodeIdx < numPayCodes; payCodeIdx++) {
for (int month = 0; month < 12; month++) {
for (int divisionIdx = 0; divisionIdx < numDivisions; divisionIdx++) {
sum = sum + transaction[month][payCodeIdx][divisionIdx];
}
}
}
// BEFORE: Complex inline test obscures main flow
if (((('a' <= inputChar) && (inputChar <= 'z')) ||
(('A' <= inputChar) && (inputChar <= 'Z'))) &&
(inputChar != 'X') && (inputChar != 'x')) {
processLetter(inputChar);
}
// AFTER: Named function (even for single use!)
// "Putting a test into a well-named function improves readability,
// and that's a sufficient reason to do it." [KEY POINT p.433]
bool isProcessableLetter(char c) {
bool isLetter = (('a' <= c) && (c <= 'z')) ||
(('A' <= c) && (c <= 'Z'));
bool isExcluded = (c == 'X') || (c == 'x');
return isLetter && !isExcluded;
}
if (isProcessableLetter(inputChar)) {
processLetter(inputChar);
}
// Range checks: order smallest → variable → largest
if (MIN_VALUE <= count && count <= MAX_VALUE) // count is between
// Out-of-range: variable on outside
if (count < MIN_VALUE || MAX_VALUE < count) // count is outside
// This visual mapping aids comprehension [KEY POINT p.440]
// Boolean: compare implicitly
while (!done) // not: while (done == false)
// Numeric: compare explicitly
while (balance != 0) // not: while (balance)
// Character (C): compare to null terminator explicitly
while (*charPtr != '\0') // not: while (*charPtr)
// Pointer: compare to NULL explicitly
while (bufferPtr != NULL) // not: while (bufferPtr)
// BEFORE: Duplicated code that will break under maintenance [ANTI-PATTERN]
GetNextRating(&ratingIncrement);
rating = rating + ratingIncrement;
while ((score < targetScore) && (ratingIncrement != 0)) {
GetNextScore(&scoreIncrement);
score = score + scoreIncrement;
GetNextRating(&ratingIncrement); // Duplicated!
rating = rating + ratingIncrement; // Duplicated!
}
// AFTER: Loop-with-exit eliminates duplication
while (true) {
GetNextRating(&ratingIncrement);
rating = rating + ratingIncrement;
if (!((score < targetScore) && (ratingIncrement != 0))) {
break;
}
GetNextScore(&scoreIncrement);
score = score + scoreIncrement;
}
// Students scored 25% higher on comprehension with this pattern [Soloway 1983]
1. Is iteration count known ahead of time?
YES → Use `for` or `foreach`
NO → Use `while`
2. Must body execute at least once?
YES → Use loop with test at END (`do-while`) or MIDDLE
NO → Use loop with test at BEGINNING (`while`)
3. Simple iteration through container?
YES → Use `foreach` (eliminates housekeeping arithmetic errors)
NO → Use appropriate `for` or `while`
4. Need to exit from middle of loop?
YES → Consider loop-with-exit (`while(true)` + `break`)
NO → Standard loop structure
These techniques are ordered from least to most invasive changes—not a strict "try #1 before #2" sequence. Choose based on what fits the problem:
Note: #5 and #6 are context-dependent, not ranked. Tables can be simpler than polymorphism (see "The fact that a design uses inheritance and polymorphism doesn't make it a good design" [p.423]).
Consider tables instead of logic when:
"The fact that a design uses inheritance and polymorphism doesn't make it a good design." [p.423]
1. Can data key directly into table? (e.g., month 1-12, char code 0-255)
YES → Direct Access
NO → Continue
2. Is keyspace large/sparse but entries few? (e.g., 100 items with 4-digit part numbers)
YES → Indexed Access (index table + main table)
NO → Continue
3. Are entries valid for ranges, not distinct points? (e.g., grade thresholds)
YES → Stair-Step Access (loop through range boundaries)
NO → Reconsider if table-driven is appropriate
// BEFORE: 20 routines, one per message type [ANTI-PATTERN]
void processMessage(Message& msg) {
if (msg.type == CYCLIC_REDUNDANCY_CHECK) {
processCRC(msg);
} else if (msg.type == BUOY_TEMPERATURE) {
processBuoyTemp(msg);
} else if (msg.type == SONOBUOY_PING) {
processSonobuoyPing(msg);
}
// ... 17 more branches
}
// AFTER: One generic routine + table definition [p.418-420]
// Table describes message format; routine interprets any message
struct FieldDef { FieldType type; string name; };
struct MessageDef { string name; vector<FieldDef> fields; };
map<MessageType, MessageDef> messageTable = {
{BUOY_TEMPERATURE, {"Buoy Temperature", {
{FLOAT, "Average Temperature"},
{FLOAT, "Temperature Range"},
{INT, "Number of Samples"},
{STRING, "Location"},
{TIME, "Time of Measurement"}
}}}
// New message types = new table entries, zero code changes
};
void processMessage(Message& msg) {
auto& def = messageTable[msg.type];
for (auto& field : def.fields) {
readAndPrint(field, msg); // One routine handles all types
}
}
How to count decision points in a routine:
1. Start with 1 for the straight path through the routine
2. Add 1 for each: if, while, repeat, for, and, or
3. Add 1 for each case in a case/switch statement
Interpretation:
0-5 → Routine is probably fine
6-10 → Start thinking about simplification
10-20 → Strong case needed to keep as-is; review for extraction
20+ → Mandatory refactor (no exceptions)
Exception criteria for 10-20 range: A routine may legitimately exceed 10 ONLY when ALL of:
NOT a valid exception: "It's just a big if-else chain" or "The complexity is inherent."
When production is down and you must fix immediately, you still MUST:
tempFix guarantees confusion laterAfter crisis resolved: Return within 24 hours to apply full control flow review. Crisis fixes that "work" often hide deeper issues.
These patterns weren't covered in Code Complete (2004) but follow the same principles:
// BEFORE: Callback hell (deep nesting equivalent)
getData(url, (data) => {
process(data, (result) => {
save(result, (saved) => {
notify(saved, () => { /* ... */ });
});
});
});
// AFTER: Flattened with async/await
const data = await getData(url);
const result = await process(data);
const saved = await save(result);
await notify(saved);
Same principle as guard clauses: flatten the structure.
// Pattern matching can exceed McCabe threshold but remain clear
// because it's flat dispatch with exhaustiveness checking
match status {
Status::Pending => handle_pending(),
Status::Active => handle_active(),
Status::Suspended => handle_suspended(),
Status::Closed => handle_closed(),
}
This is the "valid exception" for McCabe—flat dispatch with ≤3 lines per case.
// Functional pipelines replace explicit loops
const result = items
.filter(item => item.active)
.map(item => transform(item))
.reduce((acc, item) => acc + item.value, 0);
Prefer over explicit loops when: operations are independent, no early exit needed, and pipeline reads clearly.
| After | Next |
|---|---|
| Control flow verified | code-clarity-and-docs (CHECKER) |