Comprehensive guide to prevent unmaintainable code, especially from AI generation. Covers code duplication, documentation, error handling, naming conventions, architecture, performance, dependencies, modularity, testing, and technical debt prevention. Essential for code reviews and ensuring long-term code health.
This skill establishes maintainability as a critical quality attribute in code generation and review. It recognizes that AI-generated code often prioritizes immediate functionality over long-term maintainability, creating technical debt that compounds over time. This skill provides comprehensive guidelines, checklists, and best practices to ensure all code—especially AI-generated—is clean, readable, testable, and maintainable.
AI models optimize for "working code" and "completing the task" before "maintainable architecture." This creates patterns that are functional today but become liabilities tomorrow.
Functionality over Structure: AI generates code that passes immediate tests but lacks proper architecture, creating monolithic functions and tight coupling.
Pattern Repetition from Training Data: AI learns from codebases with varying quality standards, inheriting bad practices like magic numbers, poor naming, and spaghetti code.
Lack of Context Awareness: AI doesn't understand your team's conventions, existing architecture, or long-term maintenance needs.
Boilerplate Bloat: Studies show AI generates code with 8x more duplication than human developers, creating maintenance nightmares.
Happy Path Bias: AI focuses on successful scenarios, neglecting error handling, edge cases, and failure modes.
These rules must NEVER be violated, regardless of time pressure or "temporary" solutions:
No Code Duplication: Don't repeat logic—extract to reusable functions, utilities, or shared components.
Document the "Why", Not the "What": Comments should explain business decisions and rationale, not restate the code.
Handle All Edge Cases: Every function must consider null values, empty inputs, boundary conditions, and failures.
Consistent Naming: Use clear, descriptive names that reveal intent. Avoid abbreviations and single-letter variables.
Single Responsibility: Each function, class, and module should have one reason to change.
Test Everything: All code paths—including error conditions—must have automated tests.
No Hardcoded Values: Use constants, configuration, or environment variables for values that might change.
The Problem: AI generates duplicated code blocks at 8x the rate of human developers. This creates maintenance overhead, increases bug propagation, and violates the DRY (Don't Repeat Yourself) principle.
Detect duplicate blocks: Search for functions with 90%+ identical logic
Identify unnecessary layers: Review wrapper functions that add no value
processData() that only calls coreProcessData() with same argumentsRemove dead code: Eliminate unused artifacts
Measure code volume: Compare against human-written equivalents
# ❌ NEVER DO THIS - Duplicated logic
def process_user_data(data):
if data is None:
return None
cleaned = data.strip().lower()
validated = validate_length(cleaned, 100)
return validated
def process_product_data(data):
if data is None:
return None
cleaned = data.strip().lower()
validated = validate_length(cleaned, 100)
return validated
# ✅ DO THIS INSTEAD - Reusable function
def normalize_text(data, max_length=100):
"""
Normalize text input by cleaning and validating.
Business Rule: All text inputs must be normalized before storage
to ensure consistency in search and display.
Args:
data: Raw text input
max_length: Maximum allowed length (default: 100)
Returns:
Normalized string or None if input is None
"""
if data is None:
return None
cleaned = data.strip().lower()
return validate_length(cleaned, max_length)
# Usage
user_name = normalize_text(raw_user_name)
product_name = normalize_text(raw_product_name)
// ❌ NEVER DO THIS - Unnecessary wrapper
function processData(data) {
return coreProcessData(data); // No transformation, no value added
}
// ✅ DO THIS INSTEAD - Use core function directly
// Or add meaningful transformation:
function processAndValidateData(data) {
const processed = coreProcessData(data);
return validateBusinessRules(processed);
}
The Problem: AI generates comments that merely restate what the code does, providing zero value. Missing context about business decisions, edge cases, and integration points makes maintenance difficult.
Explain the "Why": Every non-obvious decision needs explanation
Document business context: Include domain knowledge
Document integration points: Explain external dependencies
Document edge cases: Explicitly call out special handling
Useful vs. useless comments: Ensure comments add value
// increment counter (restates code)// API retry limit: max 3 attempts with exponential backoff starting at 100ms# ❌ NEVER DO THIS - Useless comment
# increment counter
counter += 1
# ❌ NEVER DO THIS - Restating the obvious
def calculate_total(price, quantity):
# multiply price by quantity
return price * quantity
# ✅ DO THIS INSTEAD - Explain business context
def calculate_total(price, quantity):
"""
Calculate order total with volume discount rules.
Business Rules:
- Base price * quantity for orders < 10 items
- 5% discount for orders 10-49 items
- 10% discount for orders 50+ items
Args:
price: Unit price in cents (integer to avoid floating point errors)
quantity: Number of items ordered
Returns:
Total amount in cents
Edge Cases:
- Returns 0 if quantity is 0
- Raises ValueError if price or quantity is negative
"""
if quantity == 0:
return 0
base_total = price * quantity
if quantity >= 50:
return int(base_total * 0.90) # 10% discount
elif quantity >= 10:
return int(base_total * 0.95) # 5% discount
return base_total
// ✅ DO THIS INSTEAD - Document integration context
/**
* Process payment through Stripe integration.
*
* Integration Notes:
* - Requires STRIPE_SECRET_KEY environment variable
* - Webhook endpoint must be configured at /webhooks/stripe
* - Idempotency key prevents duplicate charges on retries
*
* Error Handling:
* - Card errors: Returns 402 with user-friendly message
* - API errors: Returns 500, logs to Sentry
* - Network errors: Retries up to 3 times with exponential backoff
*
* @param {Object} paymentData - Payment details
* @param {string} paymentData.amount - Amount in cents
* @param {string} paymentData.currency - ISO 4217 currency code
* @returns {Promise<PaymentResult>} Payment confirmation or error
*/
async function processStripePayment(paymentData) {
// Implementation with proper error handling...
}
The Problem: AI focuses on "happy path" scenarios, neglecting null checks, exceptions, and failure modes. This creates fragile code that fails unpredictably in production.
Null safety: Protect against null/undefined values
Exception handling: Catch errors at risk points
Graceful degradation: Provide fallbacks when possible
Error logging: Log appropriately for debugging
Edge case testing: Handle boundary conditions
# ❌ NEVER DO THIS - No null checks or error handling
def process_user(user):
email = user['email'] # KeyError if key missing
send_email(email) # Fails if email is None
# ✅ DO THIS INSTEAD - Comprehensive error handling
def process_user(user):
"""
Process user data with validation and error handling.
Args:
user: Dictionary containing user data
Raises:
ValueError: If user data is invalid
EmailError: If email sending fails
"""
if not user:
raise ValueError("User data is required")
email = user.get('email')
if not email:
raise ValueError("User email is required")
if not is_valid_email(email):
raise ValueError(f"Invalid email format: {email}")
try:
send_email(email)
except EmailException as e:
logger.error(f"Failed to send email to {email}: {e}")
raise EmailError(f"Could not send welcome email") from e
// ❌ NEVER DO THIS - Missing error handling
async function fetchUserData(userId) {
const response = await fetch(`/api/users/${userId}`);
const data = await response.json(); // Crashes if not valid JSON
return data;
}
// ✅ DO THIS INSTEAD - Robust error handling
async function fetchUserData(userId) {
if (!userId) {
throw new ValidationError('User ID is required');
}
try {
const response = await fetch(`/api/users/${userId}`);
if (!response.ok) {
if (response.status === 404) {
throw new NotFoundError(`User ${userId} not found`);
}
throw new ApiError(`API error: ${response.status}`);
}
const data = await response.json();
if (!data || typeof data !== 'object') {
throw new DataError('Invalid response format');
}
return data;
} catch (error) {
if (error instanceof ValidationError || error instanceof NotFoundError) {
throw error; // Re-throw known errors
}
logger.error('Failed to fetch user data', {
userId,
error: error.message,
stack: error.stack
});
throw new ServiceError('Unable to retrieve user data');
}
}
The Problem: AI generates ambiguous, inconsistent, or abbreviated names that obscure intent. Poor naming is one of the biggest barriers to code understanding.
Descriptive variable names: Reveal purpose and content
data, temp, result, value, x, yuserEmailList, calculatedTaxAmount, pendingOrderCountConsistent naming patterns: Follow conventions throughout
is, has, can, shouldAccurate naming: Names should match actual purpose
isValid should not mean isUserEmailVerifiedAvoid abbreviations: Unless universally understood
usrNm, dta, proc, calc, fnid, url, api, html (domain standards)Function naming: Use action verbs
fetch, get, create, update, delete, process, validate, calculate, transform# ❌ NEVER DO THIS - Ambiguous names
def calc(a, b):
return a * b
data = get_data()
for x in data:
process(x)
# ✅ DO THIS INSTEAD - Clear, descriptive names
def calculate_order_total(item_price, quantity):
"""Calculate total price for order line item."""
return item_price * quantity
user_orders = fetch_pending_orders()
for order in user_orders:
process_order_payment(order)
// ❌ NEVER DO THIS - Inconsistent naming and abbreviations
let usrNm = getUserName();
let is_valid = validateEmail(email);
function procData(d) { /* ... */ }
// ✅ DO THIS INSTEAD - Consistent, clear naming
const userName = getUserName();
const isEmailValid = validateEmail(email);
function processUserData(userData) { /* ... */ }
// Boolean naming convention
const isAuthenticated = checkAuthStatus();
const hasPermission = verifyUserPermission('admin');
const canEdit = determineEditAccess(document);
The Problem: AI generates code with excessive coupling, mixing concerns, and inconsistent patterns. This makes changes ripple through the codebase and violates SOLID principles.
Respect abstractions: Don't bypass interfaces
Loose coupling: Minimize inter-module dependencies
Consistent patterns: Use same approach for similar problems
Single Responsibility: One reason to change per component
Separation of concerns: Keep layers distinct
# ❌ NEVER DO THIS - High coupling, mixed concerns
class UserService:
def create_user(self, data):
# Validation mixed with business logic
if not data.get('email'):
raise ValueError("Email required")
# Direct database access mixed in
db = Database.connect("localhost", "user", "pass123")
cursor = db.cursor()
cursor.execute("INSERT INTO users ...")
# Email sending mixed in
smtp = SMTP("smtp.gmail.com")
smtp.sendmail("[email protected]", data['email'], "Welcome!")
# Logging mixed in
with open("/var/log/users.log", "a") as f:
f.write(f"User created: {data['email']}\n")
# ✅ DO THIS INSTEAD - Separated concerns
class UserValidator:
"""Validates user input data."""
def validate_create_data(self, data):
if not data.get('email'):
raise ValidationError("Email is required")
if not is_valid_email(data['email']):
raise ValidationError("Invalid email format")
return data
class UserRepository:
"""Handles user data persistence."""
def __init__(self, db_connection):
self._db = db_connection
def create(self, user_data):
# Database operations only
return self._db.insert('users', user_data)
class NotificationService:
"""Handles user notifications."""
def __init__(self, email_client, logger):
self._email = email_client
self._logger = logger
def send_welcome_email(self, user_email):
self._email.send(user_email, "Welcome!", template="welcome")
self._logger.info(f"Welcome email sent to {user_email}")
class UserService:
"""Orchestrates user creation with proper separation."""
def __init__(self, validator, repository, notifications):
self._validator = validator
self._repository = repository
self._notifications = notifications
def create_user(self, user_data):
validated_data = self._validator.validate_create_data(user_data)
user = self._repository.create(validated_data)
self._notifications.send_welcome_email(user.email)
return user
The Problem: AI prioritizes clarity over efficiency, often generating code with N+1 queries, excessive I/O, and memory waste that becomes problematic at scale.
Database optimization: Efficient queries
Minimize I/O operations: Batch operations when possible
Memory efficiency: Avoid unnecessary allocations
Async/concurrency: Don't block on I/O
Algorithm efficiency: Choose appropriate algorithms
# ❌ NEVER DO THIS - N+1 query problem
users = User.query.all()
for user in users:
# This executes a query for EACH user!
orders = Order.query.filter_by(user_id=user.id).all()
process_orders(orders)
# ✅ DO THIS INSTEAD - Eager loading
# Single query with join
users_with_orders = db.session.query(User).options(
joinedload(User.orders)
).all()
for user in users_with_orders:
process_orders(user.orders) # No additional queries
// ❌ NEVER DO THIS - Sequential async operations
async function processUsers(userIds) {
const results = [];
for (const userId of userIds) {
// Each iteration waits for the previous to complete!
const user = await fetchUser(userId);
results.push(user);
}
return results;
}
// ✅ DO THIS INSTEAD - Parallel async operations
async function processUsers(userIds) {
// All requests fire in parallel
const promises = userIds.map(userId => fetchUser(userId));
return await Promise.all(promises);
}
// ✅ Or with concurrency limiting for large batches
async function processUsersBatched(userIds, batchSize = 10) {
const results = [];
for (let i = 0; i < userIds.length; i += batchSize) {
const batch = userIds.slice(i, i + batchSize);
const batchResults = await Promise.all(
batch.map(id => fetchUser(id))
);
results.push(...batchResults);
}
return results;
}
The Problem: AI may suggest packages that don't exist, use loose version constraints, or create dependency bloat with deep transitive dependency trees.
Verify dependencies: Ensure packages exist and are legitimate
Lock dependencies: Use lockfiles for reproducible builds
package-lock.json (npm)yarn.lock (yarn)poetry.lock (Poetry)Cargo.lock (Rust)go.sum (Go)Version constraints: Use appropriate versioning
Minimize dependencies: Reduce attack surface
Audit regularly: Check for vulnerabilities
npm audit, pip-audit, snyk testnpm audit, safety check, snyk test// ❌ NEVER DO THIS - Loose versions and unchecked packages
{
"dependencies": {
"express": "*",
"unknown-package": "^1.0.0", // Might not exist!
"left-pad": "1.0.0" // Check if really needed
}
}
// ✅ DO THIS INSTEAD - Specific versions with lockfile
{
"dependencies": {
"express": "^4.18.2",
"lodash": "^4.17.21"
}
}
// Always include package-lock.json in version control
The Problem: AI generates code with unclear module boundaries, exposing internal details and creating fragile dependencies between modules.
Clear module boundaries: Changes in one module don't affect others unexpectedly
Encapsulation: Hide internal state
Single responsibility: One reason to change
Consistent interfaces: Similar modules have similar interfaces
# ❌ NEVER DO THIS - Poor encapsulation
class BankAccount:
def __init__(self):
self.balance = 0 # Public field, can be modified directly
self.transactions = [] # Internal state exposed
account = BankAccount()
account.balance = 1000000 # Anyone can modify!
# ✅ DO THIS INSTEAD - Proper encapsulation
class BankAccount:
def __init__(self):
self._balance = 0 # Private field
self._transactions = [] # Private
@property
def balance(self):
"""Read-only access to balance."""
return self._balance
def deposit(self, amount):
"""Deposit funds with validation."""
if amount <= 0:
raise ValueError("Deposit amount must be positive")
self._balance += amount
self._transactions.append(Transaction('deposit', amount))
def get_transaction_history(self):
"""Return copy of transaction history."""
return self._transactions.copy()
account = BankAccount()
account.deposit(100)
print(account.balance) # 100
# account.balance = 1000 # AttributeError: can't set attribute
The Problem: AI generates tests that only cover happy paths with weak assertions, providing false confidence while missing edge cases and error conditions.
Comprehensive test coverage: More than happy paths
Strong assertions: Verify specific outcomes
expect(result).toBeDefined()expect(result).toEqual(expectedValue)Test maintainability: Tests should be clear and maintainable
Test coverage goals: Aim for high coverage with quality
Integration testing: Test component interactions
# ❌ NEVER DO THIS - Weak tests with poor coverage
def test_calculate():
result = calculate(5, 10)
assert result is not None # Too weak!
def test_process_data():
data = {"name": "test"}
result = process_data(data)
assert result # Just checks truthiness
# ✅ DO THIS INSTEAD - Comprehensive tests
import pytest
class TestOrderCalculator:
"""Test suite for order total calculation."""
def test_calculate_simple_order(self):
"""Should correctly calculate order without discounts."""
result = calculate_order_total(price=1000, quantity=5)
assert result == 5000
def test_calculate_with_small_discount(self):
"""Should apply 5% discount for orders 10-49 items."""
result = calculate_order_total(price=100, quantity=10)
assert result == 950 # 1000 - 5%
def test_calculate_with_large_discount(self):
"""Should apply 10% discount for orders 50+ items."""
result = calculate_order_total(price=100, quantity=50)
assert result == 4500 # 5000 - 10%
def test_calculate_zero_quantity(self):
"""Should return 0 for empty orders."""
result = calculate_order_total(price=100, quantity=0)
assert result == 0
def test_calculate_negative_price_raises_error(self):
"""Should reject negative prices."""
with pytest.raises(ValueError, match="Price cannot be negative"):
calculate_order_total(price=-100, quantity=5)
def test_calculate_large_numbers(self):
"""Should handle maximum integer values."""
result = calculate_order_total(price=1000000, quantity=1000)
assert result == 900000000 # With 10% discount
// ✅ DO THIS INSTEAD - Well-structured JavaScript tests
describe('UserService', () => {
describe('createUser', () => {
it('should create user with valid data', async () => {
const userData = {
email: '[email protected]',
name: 'Test User'
};
const result = await userService.createUser(userData);
expect(result).toMatchObject({
id: expect.any(String),
email: '[email protected]',
name: 'Test User',
createdAt: expect.any(Date)
});
});
it('should throw ValidationError for missing email', async () => {
const userData = { name: 'Test User' };
await expect(userService.createUser(userData))
.rejects
.toThrow(ValidationError);
});
it('should throw DuplicateError for existing email', async () => {
const userData = { email: '[email protected]', name: 'Test' };
await expect(userService.createUser(userData))
.rejects
.toThrow(DuplicateError);
});
});
});
The Problem: AI generates "temporary" solutions that become permanent, accumulating technical debt through shortcuts, hacks, and quick fixes.
No TODO without tickets: Every TODO needs tracking
Avoid temporary hacks: Shortcuts become permanent
Configuration over code: Make behavior configurable
Version control discipline: Clean commit history
# ❌ NEVER DO THIS - Hardcoded magic numbers
def calculate_shipping(weight):
if weight < 10:
return 5.99 # Magic number!
elif weight < 50:
return 10.99 # Magic number!
return 25.99 # Magic number!
# ✅ DO THIS INSTEAD - Named constants
SHIPPING_TIERS = [
(10, 5.99), # (max_weight_kg, cost_usd)
(50, 10.99),
(float('inf'), 25.99)
]
def calculate_shipping(weight_kg):
"""Calculate shipping cost based on weight tiers."""
for max_weight, cost in SHIPPING_TIERS:
if weight_kg < max_weight:
return cost
return SHIPPING_TIERS[-1][1]
# ❌ NEVER DO THIS - TODO without context
# TODO: Fix this later
def process_payment(data):
pass # Not implemented
# ✅ DO THIS INSTEAD - TODO with ticket reference
# TODO(PROJ-1234): Implement Stripe payment processing
# Currently returns mock success for development
# See: https://jira.company.com/browse/PROJ-1234
def process_payment(data):
logger.warning("Using mock payment processor")
return PaymentResult(success=True, transaction_id="mock-123")
When reviewing AI-generated code, prioritize in this order:
| Problem | Red Flag |
|---|---|
| Duplication | Same 5+ lines in 2+ places |
| Documentation | No comments explaining "why" |
| Error Handling | Missing try/catch in risky operations |
| Edge Cases | Tests only with "happy path" values |
| Naming | Variables named data, temp, result |
| Coupling | Changing X breaks unrelated Y |
| Performance | Queries in loops, no pagination |
| Modularity | Module handles 5+ responsibilities |
| Testing | <70% code coverage |
| Dependencies | 3+ levels of transitive imports |
AI is excellent for generating functional boilerplate quickly, but requires expert review for maintainability. Treat AI-generated code as junior developer code: fast and functional, but needs architect review.
Key Principles:
Remember: Code is read 10x more than it's written. Optimize for readability and maintainability over cleverness or brevity.
This skill should be consulted for every code generation task, every code review, and every refactoring effort. Maintainability is a competitive advantage.