Engineering Code Quality

Code Quality & Maintainability

Scope: Comprehensive guide to code quality principles, SOLID design, code smells, metrics, and long-term maintainability Lines: ~400 Last Updated: 2025-10-27 Format Version: 1.0 (Atomic)

When to Use This Skill

Activate this skill when:

• Evaluating code quality in reviews or audits
• Establishing quality standards for a team
• Refactoring legacy code to improve maintainability
• Setting up code quality metrics and monitoring
• Training engineers on clean code principles
• Defining "done" criteria for features
• Improving codebase health over time
• Reducing bug rates and technical debt

Core Concepts

Concept 1: The SOLID Principles

S - Single Responsibility Principle

A class/function should have one reason to change

# Bad: User class does too much
class User:
    def save_to_database(self): ...
    def send_email(self): ...
    def generate_pdf_report(self): ...
    def validate_password(self): ...

# Good: Each class has one responsibility
class User:
    def validate_password(self): ...

class UserRepository:
    def save(self, user: User): ...

class EmailService:
    def send_welcome_email(self, user: User): ...

class ReportGenerator:
    def generate_user_report(self, user: User): ...

O - Open/Closed Principle

Open for extension, closed for modification

# Bad: Must modify class to add new payment method
class PaymentProcessor:
    def process(self, method: str, amount: float):
        if method == "credit_card":
            # Credit card logic
        elif method == "paypal":
            # PayPal logic
        elif method == "bitcoin":  # Must modify existing code!
            # Bitcoin logic

# Good: Extend via new classes
class PaymentMethod:
    def process(self, amount: float): ...

class CreditCardPayment(PaymentMethod):
    def process(self, amount: float): ...

class PayPalPayment(PaymentMethod):
    def process(self, amount: float): ...

class PaymentProcessor:
    def process(self, method: PaymentMethod, amount: float):
        method.process(amount)  # No modification needed!

L - Liskov Substitution Principle

Subtypes must be substitutable for their base types

// Bad: Violates LSP
class Rectangle {
  width: number;
  height: number;
  setWidth(w: number) { this.width = w; }
  setHeight(h: number) { this.height = h; }
  area(): number { return this.width * this.height; }
}

class Square extends Rectangle {
  setWidth(w: number) {
    this.width = w;
    this.height = w;  // Breaks expected Rectangle behavior!
  }
}

// Good: Composition over inheritance
interface Shape {
  area(): number;
}

class Rectangle implements Shape {
  constructor(private width: number, private height: number) {}
  area(): number { return this.width * this.height; }
}

class Square implements Shape {
  constructor(private side: number) {}
  area(): number { return this.side * this.side; }
}

I - Interface Segregation Principle

Clients shouldn't depend on interfaces they don't use

// Bad: Fat interface
type Worker interface {
    Work()
    Eat()
    Sleep()
}

type Robot struct{}

func (r Robot) Work() { /* ... */ }
func (r Robot) Eat() { /* Robots don't eat! */ }
func (r Robot) Sleep() { /* Robots don't sleep! */ }

// Good: Segregated interfaces
type Worker interface {
    Work()
}

type LivingWorker interface {
    Worker
    Eat()
    Sleep()
}

type Robot struct{}
func (r Robot) Work() { /* ... */ }

type Human struct{}
func (h Human) Work() { /* ... */ }
func (h Human) Eat() { /* ... */ }
func (h Human) Sleep() { /* ... */ }

D - Dependency Inversion Principle

Depend on abstractions, not concretions

// Bad: High-level module depends on low-level module
class MySQLDatabase {
  save(data: string) { /* MySQL specific */ }
}

class UserService {
  private db = new MySQLDatabase();  // Tight coupling!

  saveUser(user: User) {
    this.db.save(JSON.stringify(user));
  }
}

// Good: Both depend on abstraction
interface Database {
  save(data: string): void;
}

class MySQLDatabase implements Database {
  save(data: string) { /* MySQL specific */ }
}

class PostgresDatabase implements Database {
  save(data: string) { /* Postgres specific */ }
}

class UserService {
  constructor(private db: Database) {}  // Depend on interface

  saveUser(user: User) {
    this.db.save(JSON.stringify(user));
  }
}

Concept 2: Code Smells

Common Code Smells:

Examples:

# Smell: Magic Numbers
def calculate_price(quantity):
    return quantity * 19.99 * 1.08  # What are these?

# Fixed: Named Constants
PRICE_PER_ITEM = 19.99
TAX_RATE = 1.08

def calculate_price(quantity):
    return quantity * PRICE_PER_ITEM * TAX_RATE

// Smell: Long Parameter List
function createUser(
  name: string,
  email: string,
  age: number,
  address: string,
  phone: string,
  company: string
) { }

// Fixed: Parameter Object
interface UserData {
  name: string;
  email: string;
  age: number;
  address: string;
  phone: string;
  company: string;
}

function createUser(data: UserData) { }

// Smell: Nested Conditionals
func processOrder(order Order) error {
    if order.IsValid() {
        if order.HasInventory() {
            if order.PaymentSucceeded() {
                if order.ShippingAvailable() {
                    // Process order
                }
            }
        }
    }
}

// Fixed: Guard Clauses
func processOrder(order Order) error {
    if !order.IsValid() {
        return ErrInvalidOrder
    }
    if !order.HasInventory() {
        return ErrOutOfStock
    }
    if !order.PaymentSucceeded() {
        return ErrPaymentFailed
    }
    if !order.ShippingAvailable() {
        return ErrShippingUnavailable
    }

    // Process order
    return nil
}

Concept 3: Code Quality Metrics

Cyclomatic Complexity

Measure of code complexity based on number of independent paths

# Complexity: 1 (simple)
def add(a, b):
    return a + b

# Complexity: 4 (moderate)
def get_discount(user_type, purchase_amount):
    if user_type == "premium":
        if purchase_amount > 100:
            return 0.20
        return 0.10
    elif user_type == "regular":
        if purchase_amount > 100:
            return 0.10
    return 0

# Target: Keep functions < 10 complexity

Code Coverage

Percentage of code executed by tests

# Good coverage targets:
# - Critical business logic: 90%+
# - General codebase: 70-80%
# - UI/Glue code: 50-60%

pytest --cov=myapp --cov-report=html
# coverage: 78% (good!)

Maintainability Index

Composite metric: 171 - 5.2 * ln(V) - 0.23 * G - 16.2 * ln(L)

• V: Halstead Volume (complexity)
• G: Cyclomatic Complexity
• L: Lines of Code

Code Churn

How often files change

# High churn = potential quality issues
git log --format=format: --name-only | sort | uniq -c | sort -rn | head -10

# Example output:
#  47 src/utils/helpers.py  # Too much churn!
#  12 src/models/user.py    # Acceptable

Patterns

Pattern 1: Readable Code Structure

Function Length:

# Bad: Long function (100+ lines)
def process_order(order):
    # 100 lines of mixed concerns

# Good: Small, focused functions
def process_order(order):
    validate_order(order)
    charge_payment(order)
    update_inventory(order)
    send_confirmation(order)

# Each function < 20 lines, single responsibility

Variable Naming:

// Bad: Unclear names
let d: number;  // What is d?
let tmp: string;  // Temporary what?
let data: any[];  // What kind of data?

// Good: Descriptive names
let daysUntilExpiration: number;
let userEmail: string;
let activeOrders: Order[];

Comments:

// Bad: Redundant comment
// Increment i by 1
i++

// Bad: Outdated comment
// Calculate tax rate (7%)
taxRate := 0.08  // Comment is wrong!

// Good: Explain WHY, not WHAT
// Use exponential backoff to avoid overwhelming the API
// during high traffic periods
time.Sleep(time.Duration(math.Pow(2, retries)) * time.Second)

Pattern 2: Error Handling

Python:

# Bad: Silently swallowing errors
try:
    user = get_user(user_id)
except:
    pass  # What happened?

# Good: Specific error handling
try:
    user = get_user(user_id)
except UserNotFoundError as e:
    logger.error(f"User {user_id} not found: {e}")
    raise
except DatabaseError as e:
    logger.error(f"Database error: {e}")
    # Try fallback
    user = get_user_from_cache(user_id)

Go:

// Bad: Ignoring errors
user, _ := getUser(userID)  // What if it fails?

// Good: Explicit error handling
user, err := getUser(userID)
if err != nil {
    return fmt.Errorf("failed to get user %d: %w", userID, err)
}

Rust:

// Bad: Unwrapping everywhere
let user = get_user(user_id).unwrap();  // Panics on error!

// Good: Propagating errors
let user = get_user(user_id)?;  // Returns early if error

// Or: Pattern matching
match get_user(user_id) {
    Ok(user) => process_user(user),
    Err(e) => log::error!("Failed to get user: {}", e),
}

Pattern 3: Testable Code Design

Bad: Hard to Test:

class UserService {
  async createUser(email: string): Promise<User> {
    // Hard-coded dependency!
    const db = new MySQLDatabase();
    const emailService = new SendGridEmailService();
    const logger = new FileLogger();

    const user = await db.save({ email });
    await emailService.send(user.email, "Welcome!");
    logger.info(`Created user ${user.id}`);

    return user;
  }
}

// Can't test without real database, email service, file system!

Good: Easy to Test:

interface Database {
  save(data: any): Promise<User>;
}

interface EmailService {
  send(to: string, subject: string): Promise<void>;
}

interface Logger {
  info(message: string): void;
}

class UserService {
  constructor(
    private db: Database,
    private emailService: EmailService,
    private logger: Logger
  ) {}

  async createUser(email: string): Promise<User> {
    const user = await this.db.save({ email });
    await this.emailService.send(user.email, "Welcome!");
    this.logger.info(`Created user ${user.id}`);
    return user;
  }
}

// Easy to test with mocks!
const mockDb = { save: jest.fn() };
const mockEmail = { send: jest.fn() };
const mockLogger = { info: jest.fn() };
const service = new UserService(mockDb, mockEmail, mockLogger);

Pattern 4: Code Organization

Package Structure:

# Bad: Organized by type
src/
  controllers/
    user_controller.py
    order_controller.py
    product_controller.py
  models/
    user.py
    order.py
    product.py
  services/
    user_service.py
    order_service.py
    product_service.py

# Good: Organized by feature/domain
src/
  users/
    controller.py
    model.py
    service.py
    repository.py
  orders/
    controller.py
    model.py
    service.py
    repository.py
  products/
    controller.py
    model.py
    service.py
    repository.py

Module Cohesion:

# Bad: Low cohesion - unrelated functions
def calculate_tax(amount): ...
def send_email(to, subject): ...
def hash_password(password): ...

# Good: High cohesion - related functions
# tax_calculator.py
def calculate_sales_tax(amount, state): ...
def calculate_income_tax(income, bracket): ...
def get_tax_rate(state): ...

# email_service.py
def send_email(to, subject, body): ...
def send_bulk_email(recipients, subject, body): ...
def validate_email(email): ...

# auth_service.py
def hash_password(password): ...
def verify_password(password, hash): ...
def generate_token(user_id): ...

Best Practices

Code Quality Checklist

Before Committing:

• No commented-out code (delete it, git remembers)
• No TODO comments (create tickets instead)
• No magic numbers (use named constants)
• No functions > 30 lines
• No classes > 500 lines
• No duplicate code (DRY principle)
• Meaningful variable/function names
• Complex logic has comments explaining WHY

During Review:

• Follows team's style guide
• Has adequate test coverage
• No obvious bugs or edge cases
• Error handling is appropriate
• Dependencies are necessary
• Performance is acceptable
• Security vulnerabilities addressed

Anti-Patterns

Common Mistakes

❌ Over-engineering: Premature abstraction
→ Start simple, refactor when needed

❌ God objects: Classes that do everything
→ Follow Single Responsibility Principle

❌ Premature optimization: Optimizing before profiling
→ Make it work, make it right, make it fast

❌ Copy-paste coding: Duplicating logic
→ Extract to shared functions

❌ Commenting obvious code: // Set x to 5
→ Only comment complex/non-obvious logic

❌ Inconsistent naming: getUserData() vs fetchUser()
→ Pick conventions and stick to them

❌ Deep nesting: If/else 5+ levels deep
→ Use guard clauses, early returns

❌ Ignoring errors: try { } catch { }
→ Handle errors explicitly

Tools & Automation

Static Analysis Tools

Python:

# Linting
flake8 src/
pylint src/

# Type checking
mypy src/

# Code quality
radon cc src/ -a  # Cyclomatic complexity
radon mi src/     # Maintainability index

# Security
bandit -r src/

JavaScript/TypeScript:

# Linting
eslint src/

# Type checking
tsc --noEmit

# Code quality
npx complexity-report src/

Go:

# Linting
golangci-lint run

# Cyclomatic complexity
gocyclo -over 10 .

# Security
gosec ./...

Quality Gates in CI

# .github/workflows/quality.yml
name: Code Quality
on: [pull_request]

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - name: Run linters
        run: |
          flake8 src/ --max-complexity=10
          pylint src/ --fail-under=8.0

      - name: Check test coverage
        run: |
          pytest --cov=src --cov-fail-under=80

      - name: Security scan
        run: |
          bandit -r src/ -ll

      - name: Check complexity
        run: |
          radon cc src/ -a -nb --total-average-threshold=B

Related Skills

• engineering-code-review: Reviewing code for quality issues
• engineering-refactoring-patterns: Improving code quality through refactoring
• engineering-test-driven-development: Writing quality code via TDD
• engineering-technical-debt: Managing quality trade-offs
• engineering-design-patterns: Applying proven design solutions

References

• Clean Code by Robert C. Martin
• SOLID Principles
• Code Smells Catalog
• Cyclomatic Complexity