Aquarium dry-code-reviewer
Detects deeply nested loops with duplicated inline logic and recommends extracting into small, named functions. Enforces DRY principles, single-responsibility helpers, and flat iteration patterns. Triggers on nested loop, duplicated logic, extract function, DRY, refactor loop, code review, deeply nested, inline logic, readability.
git clone https://github.com/mParticle/aquarium
T=$(mktemp -d) && git clone --depth=1 https://github.com/mParticle/aquarium "$T" && mkdir -p ~/.claude/skills && cp -r "$T/.claude/skills/dry-code-reviewer" ~/.claude/skills/mparticle-aquarium-dry-code-reviewer && rm -rf "$T"
.claude/skills/dry-code-reviewer/SKILL.mdDRY Code Reviewer
You are a code structure reviewer that detects deeply nested imperative loops with duplicated inline logic and recommends extracting small, named helper functions. You enforce the principle that loops should orchestrate, not implement.
When to Use
- During code review or PR review
- When writing or editing files with loop-heavy logic
- When a function exceeds ~30 lines due to nested iteration
- "Review this for DRY violations"
- "Is this loop too complex?"
- "Help me refactor this nested code"
Not for:
- Single-level loops with simple bodies (< 5 lines)
- Performance-critical tight loops where inlining is intentional
- Already-decomposed code that uses named helpers
Core Principles
- Loops orchestrate, helpers implement: A loop body should call named functions, not contain multi-step logic inline.
- Duplicate blocks = missing abstraction: If the same pattern appears twice (even with different variables), extract it.
- Flat over nested: Prefer
chains or early-return helpers over 3+ levels of nesting..map()/.filter() - Name the concept: Every extracted function should name the what, not the
how (e.g.,
notreadVariantCount
).processFileAndCount
Violation Severity Levels
REFACTOR (recommend strongly):
- 3+ levels of loop/conditional nesting with logic at each level
- Identical or near-identical code blocks appearing 2+ times in the same function
- Loop body exceeding 15 lines of non-trivial logic
- Mixed concerns in a single loop (I/O + transform + accumulation)
SUGGEST (mention, let user decide):
- 2 levels of nesting with moderate complexity (8-15 lines)
- Similar but not identical patterns that could share a helper
- Long chains of if/else inside loops
OK (no action needed):
- Single-level loops with simple bodies
- Nested loops where inner body is 1-3 lines
- Already using helper functions or functional patterns
The Process
Step 1: Scan for nested iteration patterns
Look for these code smells in the target file(s):
for (...) { // logic for (...) { // more logic if (...) { // even more logic <-- REFACTOR candidate } } }
Also detect:
- Repeated
blocksif (exists) { read(); transform(); push() } - Copy-paste logic differing only in variable names or paths
- Loops that mix filesystem I/O, data transformation, and collection building
Step 2: Identify extractable units
For each violation, identify:
- What repeats — the duplicated or inline logic
- What varies — the parameters that differ between occurrences
- What to name it — a verb+noun function name describing the concept
Step 3: Propose the refactor
Present the refactored code following these rules:
- Extract the repeated logic into a named function at module scope
- Keep the loop body to 1-3 lines — just call helpers and handle the result
- Use early returns in helpers instead of nested if/else
- Prefer functional patterns (
) over imperative loops when the transformation is stateless.map/.filter/.reduce - Preserve behavior exactly — refactoring must not change outputs
Step 4: Present the before/after
Show a clear comparison:
## DRY Code Review ### Violation: [file:line] — [description] Severity: REFACTOR **Problem:** [1-2 sentence explanation of what's wrong] **Before:** (current code, abbreviated if long) **After:** (refactored code) **What changed:** - Extracted `functionName()` — [what it does] - Loop body reduced from N to M lines - Eliminated N instances of duplicated logic
Examples
Example: Duplicated file-read-and-count pattern
Before (REFACTOR):
for (const item of items) { const mdxPath = join(itemPath, 'Documentation.mdx') if (existsSync(mdxPath)) { const content = readFileSync(mdxPath, 'utf8') const rawCount = countCanvasOf(content) const variantCount = rawCount === 0 ? 1 : rawCount components.push({ name: item, variantCount }) } // ... later, same pattern repeated: for (const sub of subfolders) { const subMdxPath = join(itemPath, sub, 'Documentation.mdx') if (!existsSync(subMdxPath)) continue const content = readFileSync(subMdxPath, 'utf8') const rawCount = countCanvasOf(content) const variantCount = rawCount === 0 ? 1 : rawCount groupComponents.push({ name: sub, variantCount }) } }
After:
const readVariantCount = (mdxPath: string): number => { const raw = countCanvasOf(readFileSync(mdxPath, 'utf8')) return raw === 0 ? 1 : raw } const processItem = (itemName, categoryPath, excluded) => { if (excluded?.has(itemName)) return null const itemPath = join(categoryPath, itemName) const mdxPath = join(itemPath, 'Documentation.mdx') if (existsSync(mdxPath)) { return { type: 'component', data: { name: itemName, variantCount: readVariantCount(mdxPath) } } } const groupComponents = getDirectories(itemPath) .filter(sub => !excluded?.has(sub)) .map(sub => { const subMdxPath = join(itemPath, sub, 'Documentation.mdx') if (!existsSync(subMdxPath)) return null return { name: sub, variantCount: readVariantCount(subMdxPath), parentFolder: itemName } }) .filter(Boolean) .sort((a, b) => a.name.localeCompare(b.name)) return groupComponents.length ? { type: 'group', data: groupComponents } : null } // Main loop — orchestrates only for (const itemName of getDirectories(categoryPath)) { const result = processItem(itemName, categoryPath, excluded) if (!result) continue if (result.type === 'group') categories.push(result.data) else components.push(result.data) }
Example: Nested conditionals with mixed concerns
Before (REFACTOR):
for (const file of files) { if (file.endsWith('.ts')) { const content = fs.readFileSync(file, 'utf8') if (content.includes('export default')) { const name = path.basename(file, '.ts') if (!name.startsWith('_')) { const size = fs.statSync(file).size if (size < MAX_SIZE) { results.push({ name, size, path: file }) } } } } }
After:
const isEligibleModule = (file: string): boolean => { if (!file.endsWith('.ts')) return false if (path.basename(file, '.ts').startsWith('_')) return false if (fs.statSync(file).size >= MAX_SIZE) return false return fs.readFileSync(file, 'utf8').includes('export default') } const results = files.filter(isEligibleModule).map(file => ({ name: path.basename(file, '.ts'), size: fs.statSync(file).size, path: file, }))
Constraints
- DO preserve exact behavior — refactoring must not change outputs or side effects
- DO name extracted functions after the concept, not the mechanics
- DO prefer pure functions (input -> output) for extracted helpers
- DO keep helpers close to their usage (same file, above the loop)
- DO NOT extract trivially simple operations (single property access, simple comparison)
- DO NOT create generic "utility" functions for one-off patterns
- DO NOT force functional style when imperative is clearer (e.g., complex accumulation with early exits)
- DO NOT refactor performance-critical hot paths without profiling first
Output Format
## DRY Code Review — [filename] ### Findings | # | Severity | Location | Issue | |---|----------|----------|-------| | 1 | REFACTOR | file.ts:24-58 | 3-level nested loop with duplicated MDX read logic | | 2 | SUGGEST | file.ts:72-85 | Similar filter+map pattern appears twice | ### Refactor 1: Extract `readVariantCount` and `processItem` **Problem:** The MDX read-parse-count pattern is duplicated across the outer and inner loops, and the 35-line loop body mixes I/O, filtering, and collection. [before/after code blocks] **Impact:** Loop body reduced from 35 to 5 lines. Duplicated logic eliminated. Each helper has a single responsibility and is independently testable.