Code Review Checklist

Structured checklists for systematic code review across different aspects.

Table of Contents

• Pre-Review Checks
• Correctness
• Security
• Performance
• Maintainability
• Testing
• Documentation
• Language-Specific Checks

Pre-Review Checks

Before diving into code, verify these basics:

Build and Tests

• Code compiles without errors
• All existing tests pass
• New tests are included for new functionality
• No unintended files included (build artifacts, IDE configs)

PR Hygiene

• PR has clear title and description
• Changes are scoped appropriately (not too large)
• Commits follow conventional commit format
• Branch is up to date with base branch

Scope Verification

• Changes match the stated purpose
• No unrelated changes bundled in
• Breaking changes are documented
• Migration path provided if needed

Correctness

Logic

• Algorithm implements requirements correctly
• Edge cases handled (null, empty, boundary values)
• Off-by-one errors checked
• Correct operators used (== vs ===, & vs &&)
• Loop termination conditions correct
• Recursion has proper base cases

Data Handling

• Data types appropriate for the use case
• Numeric overflow/underflow considered
• Date/time handling accounts for timezones
• Unicode and internationalization handled
• Data validation at entry points

State Management

• State transitions are valid
• Race conditions addressed
• Concurrent access handled correctly
• State cleanup on errors/exit

Error Handling

• Errors caught at appropriate levels
• Error messages are actionable
• Errors don't expose sensitive information
• Recovery or graceful degradation implemented
• Resources cleaned up in error paths

Security

Input Validation

• All user input validated and sanitized
• Input length limits enforced
• File uploads validated (type, size, content)
• URL parameters validated

Injection Prevention

• SQL queries parameterized
• Command execution uses safe APIs
• HTML output escaped to prevent XSS
• LDAP queries properly escaped
• XML parsing disables external entities

Authentication & Authorization

• Authentication required for protected resources
• Authorization checked before operations
• Session management secure
• Password handling follows best practices
• Token expiration implemented

Data Protection

• Sensitive data encrypted at rest
• Sensitive data encrypted in transit
• PII handled according to policy
• Secrets not hardcoded
• Logs don't contain sensitive data

API Security

• Rate limiting implemented
• CORS configured correctly
• CSRF protection in place
• API keys/tokens secured
• Endpoints use HTTPS

Performance

Efficiency

• Appropriate data structures used
• Algorithms have acceptable complexity
• Database queries are optimized
• N+1 query problems avoided
• Indexes used where beneficial

Resource Usage

• Memory usage bounded
• No memory leaks
• File handles properly closed
• Database connections pooled
• Network calls minimized

Caching

• Appropriate caching strategy
• Cache invalidation handled
• Cache keys are unique and predictable
• TTL values appropriate

Scalability

• Horizontal scaling considered
• Bottlenecks identified
• Async processing for long operations
• Batch operations where appropriate

Maintainability

Code Quality

• Functions/methods have single responsibility
• Classes follow SOLID principles
• Code is DRY (Don't Repeat Yourself)
• No dead code or commented-out code
• Magic numbers replaced with constants

Naming

• Names are descriptive and consistent
• Naming follows project conventions
• No abbreviations that obscure meaning
• Boolean variables/functions have is/has/can prefix

Structure

• Functions are appropriately sized (<50 lines preferred)
• Nesting depth is reasonable (<4 levels)
• Related code is grouped together
• Dependencies are minimal and explicit

Readability

• Code is self-documenting where possible
• Complex logic has explanatory comments
• Formatting is consistent
• No overly clever or obscure code

Testing

Coverage

• New code has unit tests
• Critical paths have integration tests
• Edge cases are tested
• Error conditions are tested

Quality

• Tests are independent
• Tests have clear assertions
• Test names describe what is tested
• Tests don't depend on external state

Mocking

• External dependencies are mocked
• Mocks are realistic
• Mock setup is not excessive

Documentation

Code Documentation

• Public APIs are documented
• Complex algorithms explained
• Non-obvious decisions documented
• TODO/FIXME comments have context

External Documentation

• README updated if needed
• API documentation updated
• Changelog updated
• Migration guides provided

Language-Specific Checks

TypeScript/JavaScript

• Types are explicit (avoid any)
• Null checks present (?., ??)
• Async/await errors handled
• No floating promises
• Memory leaks from closures checked

Python

• Type hints used for public APIs
• Context managers for resources (with statements)
• Exception handling is specific (not bare except)
• No mutable default arguments
• List comprehensions used appropriately

Go

• Errors checked and handled
• Goroutine leaks prevented
• Context propagation correct
• Defer statements in right order
• Interfaces minimal

Swift

• Optionals handled safely
• Memory management correct (weak/unowned)
• Error handling uses Result or throws
• Access control appropriate
• Codable implementation correct

Kotlin

• Null safety leveraged
• Coroutine cancellation handled
• Data classes used appropriately
• Extension functions don't obscure behavior
• Sealed classes for state

Review Process Tips

Before Approving

1. Verify all critical checks passed
2. Confirm tests are adequate
3. Consider deployment impact
4. Check for any security concerns
5. Ensure documentation is updated

Providing Feedback

• Be specific about issues
• Explain why something is problematic
• Suggest alternatives when possible
• Distinguish blockers from suggestions
• Acknowledge good patterns

When to Block

• Security vulnerabilities present
• Critical logic errors
• No tests for risky changes
• Breaking changes without migration
• Significant performance regressions

Coding Standards

Language-specific coding standards and conventions for code review.

Table of Contents

• Universal Principles
• TypeScript Standards
• JavaScript Standards
• Python Standards
• Go Standards
• Swift Standards
• Kotlin Standards

Universal Principles

These apply across all languages.

Naming Conventions

Function Design

Good functions:
- Do one thing well
- Have descriptive names (verb + noun)
- Take 3 or fewer parameters
- Return early for error cases
- Stay under 50 lines

Error Handling

Good error handling:
- Catch specific errors, not generic exceptions
- Log with context (what, where, why)
- Clean up resources in error paths
- Don't swallow errors silently
- Provide actionable error messages

TypeScript Standards

Type Annotations

// Avoid 'any' - use unknown for truly unknown types
function processData(data: unknown): ProcessedResult {
  if (isValidData(data)) {
    return transform(data);
  }
  throw new Error('Invalid data format');
}

// Use explicit return types for public APIs
export function calculateTotal(items: CartItem[]): number {
  return items.reduce((sum, item) => sum + item.price, 0);
}

// Use type guards for runtime checks
function isUser(obj: unknown): obj is User {
  return (
    typeof obj === 'object' &&
    obj !== null &&
    'id' in obj &&
    'email' in obj
  );
}

Null Safety

// Use optional chaining and nullish coalescing
const userName = user?.profile?.name ?? 'Anonymous';

// Be explicit about nullable types
interface Config {
  timeout: number;
  retries?: number;  // Optional
  fallbackUrl: string | null;  // Explicitly nullable
}

// Use assertion functions for validation
function assertDefined<T>(value: T | null | undefined): asserts value is T {
  if (value === null || value === undefined) {
    throw new Error('Value is not defined');
  }
}

Async/Await

// Always handle errors in async functions
async function fetchUser(id: string): Promise<User> {
  try {
    const response = await api.get(`/users/${id}`);
    return response.data;
  } catch (error) {
    logger.error('Failed to fetch user', { id, error });
    throw new UserFetchError(id, error);
  }
}

// Use Promise.all for parallel operations
async function loadDashboard(userId: string): Promise<Dashboard> {
  const [profile, stats, notifications] = await Promise.all([
    fetchProfile(userId),
    fetchStats(userId),
    fetchNotifications(userId)
  ]);
  return { profile, stats, notifications };
}

React/Component Standards

// Use explicit prop types
interface ButtonProps {
  label: string;
  onClick: () => void;
  variant?: 'primary' | 'secondary';
  disabled?: boolean;
}

// Prefer functional components with hooks
function Button({ label, onClick, variant = 'primary', disabled = false }: ButtonProps) {
  return (
    <button
      className={`btn btn-${variant}`}
      onClick={onClick}
      disabled={disabled}
    >
      {label}
    </button>
  );
}

// Use custom hooks for reusable logic
function useDebounce<T>(value: T, delay: number): T {
  const [debouncedValue, setDebouncedValue] = useState(value);

  useEffect(() => {
    const timer = setTimeout(() => setDebouncedValue(value), delay);
    return () => clearTimeout(timer);
  }, [value, delay]);

  return debouncedValue;
}

JavaScript Standards

Variable Declarations

// Use const by default, let when reassignment needed
const MAX_ITEMS = 100;
let currentCount = 0;

// Never use var
// var is function-scoped and hoisted, leading to bugs

Object and Array Patterns

// Use object destructuring
const { name, email, role = 'user' } = user;

// Use spread for immutable updates
const updatedUser = { ...user, lastLogin: new Date() };
const updatedList = [...items, newItem];

// Use array methods over loops
const activeUsers = users.filter(u => u.isActive);
const emails = users.map(u => u.email);
const total = orders.reduce((sum, o) => sum + o.amount, 0);

Module Patterns

// Use named exports for utilities
export function formatDate(date) { ... }
export function parseDate(str) { ... }

// Use default export for main component/class
export default class UserService { ... }

// Group related exports
export { formatDate, parseDate, isValidDate } from './dateUtils';

Python Standards

Type Hints (PEP 484)

from typing import Optional, List, Dict, Union

def get_user(user_id: int) -> Optional[User]:
    """Fetch user by ID, returns None if not found."""
    return db.query(User).filter(User.id == user_id).first()

def process_items(items: List[str]) -> Dict[str, int]:
    """Count occurrences of each item."""
    return {item: items.count(item) for item in set(items)}

def send_notification(
    user: User,
    message: str,
    *,
    priority: str = "normal",
    channels: List[str] = None
) -> bool:
    """Send notification to user via specified channels."""
    channels = channels or ["email"]
    # Implementation

Exception Handling

# Catch specific exceptions
try:
    result = api_client.fetch_data(endpoint)
except ConnectionError as e:
    logger.warning(f"Connection failed: {e}")
    return cached_data
except TimeoutError as e:
    logger.error(f"Request timed out: {e}")
    raise ServiceUnavailableError() from e

# Use context managers for resources
with open(filepath, 'r') as f:
    data = json.load(f)

# Custom exceptions should be informative
class ValidationError(Exception):
    def __init__(self, field: str, message: str):
        self.field = field
        self.message = message
        super().__init__(f"{field}: {message}")

Class Design

from dataclasses import dataclass
from abc import ABC, abstractmethod

# Use dataclasses for data containers
@dataclass
class UserDTO:
    id: int
    email: str
    name: str
    is_active: bool = True

# Use ABC for interfaces
class Repository(ABC):
    @abstractmethod
    def find_by_id(self, id: int) -> Optional[Entity]:
        pass

    @abstractmethod
    def save(self, entity: Entity) -> Entity:
        pass

# Use properties for computed attributes
class Order:
    def __init__(self, items: List[OrderItem]):
        self._items = items

    @property
    def total(self) -> Decimal:
        return sum(item.price * item.quantity for item in self._items)

Go Standards

Error Handling

// Always check errors
file, err := os.Open(filename)
if err != nil {
    return fmt.Errorf("failed to open %s: %w", filename, err)
}
defer file.Close()

// Use custom error types for specific cases
type ValidationError struct {
    Field   string
    Message string
}

func (e *ValidationError) Error() string {
    return fmt.Sprintf("%s: %s", e.Field, e.Message)
}

// Wrap errors with context
if err := db.Query(query); err != nil {
    return fmt.Errorf("query failed for user %d: %w", userID, err)
}

Struct Design

// Use unexported fields with exported methods
type UserService struct {
    repo   UserRepository
    cache  Cache
    logger Logger
}

// Constructor functions for initialization
func NewUserService(repo UserRepository, cache Cache, logger Logger) *UserService {
    return &UserService{
        repo:   repo,
        cache:  cache,
        logger: logger,
    }
}

// Keep interfaces small
type Reader interface {
    Read(p []byte) (n int, err error)
}

type Writer interface {
    Write(p []byte) (n int, err error)
}

Concurrency

// Use context for cancellation
func fetchData(ctx context.Context, url string) ([]byte, error) {
    req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
    if err != nil {
        return nil, err
    }
    // ...
}

// Use channels for communication
func worker(jobs <-chan Job, results chan<- Result) {
    for job := range jobs {
        result := process(job)
        results <- result
    }
}

// Use sync.WaitGroup for coordination
var wg sync.WaitGroup
for _, item := range items {
    wg.Add(1)
    go func(i Item) {
        defer wg.Done()
        processItem(i)
    }(item)
}
wg.Wait()

Swift Standards

Optionals

// Use optional binding
if let user = fetchUser(id: userId) {
    displayProfile(user)
}

// Use guard for early exit
guard let data = response.data else {
    throw NetworkError.noData
}

// Use nil coalescing for defaults
let displayName = user.nickname ?? user.email

// Avoid force unwrapping except in tests
// BAD: let name = user.name!
// GOOD: guard let name = user.name else { return }

Protocol-Oriented Design

// Define protocols with minimal requirements
protocol Identifiable {
    var id: String { get }
}

protocol Persistable: Identifiable {
    func save() throws
    static func find(by id: String) -> Self?
}

// Use protocol extensions for default implementations
extension Persistable {
    func save() throws {
        try Storage.shared.save(self)
    }
}

// Prefer composition over inheritance
struct User: Identifiable, Codable {
    let id: String
    var name: String
    var email: String
}

Error Handling

// Define domain-specific errors
enum AuthError: Error {
    case invalidCredentials
    case tokenExpired
    case networkFailure(underlying: Error)
}

// Use Result type for async operations
func authenticate(
    email: String,
    password: String,
    completion: @escaping (Result<User, AuthError>) -> Void
)

// Use throws for synchronous operations
func validate(_ input: String) throws -> ValidatedInput {
    guard !input.isEmpty else {
        throw ValidationError.emptyInput
    }
    return ValidatedInput(value: input)
}

Kotlin Standards

Null Safety

// Use nullable types explicitly
fun findUser(id: Int): User? {
    return userRepository.find(id)
}

// Use safe calls and elvis operator
val name = user?.profile?.name ?: "Unknown"

// Use let for null checks with side effects
user?.let { activeUser ->
    sendWelcomeEmail(activeUser.email)
    logActivity(activeUser.id)
}

// Use require/check for validation
fun processPayment(amount: Double) {
    require(amount > 0) { "Amount must be positive: $amount" }
    // Process
}

Data Classes and Sealed Classes

// Use data classes for DTOs
data class UserDTO(
    val id: Int,
    val email: String,
    val name: String,
    val isActive: Boolean = true
)

// Use sealed classes for state
sealed class Result<out T> {
    data class Success<T>(val data: T) : Result<T>()
    data class Error(val message: String, val cause: Throwable? = null) : Result<Nothing>()
    object Loading : Result<Nothing>()
}

// Pattern matching with when
fun handleResult(result: Result<User>) = when (result) {
    is Result.Success -> showUser(result.data)
    is Result.Error -> showError(result.message)
    Result.Loading -> showLoading()
}

Coroutines

// Use structured concurrency
suspend fun loadDashboard(): Dashboard = coroutineScope {
    val profile = async { fetchProfile() }
    val stats = async { fetchStats() }
    val notifications = async { fetchNotifications() }

    Dashboard(
        profile = profile.await(),
        stats = stats.await(),
        notifications = notifications.await()
    )
}

// Handle cancellation
suspend fun fetchWithRetry(url: String): Response {
    repeat(3) { attempt ->
        try {
            return httpClient.get(url)
        } catch (e: IOException) {
            if (attempt == 2) throw e
            delay(1000L * (attempt + 1))
        }
    }
    throw IllegalStateException("Unreachable")
}

Common Antipatterns

Code antipatterns to identify during review, with examples and fixes.

Table of Contents

• Structural Antipatterns
• Logic Antipatterns
• Security Antipatterns
• Performance Antipatterns
• Testing Antipatterns
• Async Antipatterns

Structural Antipatterns

God Class

A class that does too much and knows too much.

// BAD: God class handling everything
class UserManager {
  createUser(data: UserData) { ... }
  updateUser(id: string, data: UserData) { ... }
  deleteUser(id: string) { ... }
  sendEmail(userId: string, content: string) { ... }
  generateReport(userId: string) { ... }
  validatePassword(password: string) { ... }
  hashPassword(password: string) { ... }
  uploadAvatar(userId: string, file: File) { ... }
  resizeImage(file: File) { ... }
  logActivity(userId: string, action: string) { ... }
  // 50 more methods...
}

// GOOD: Single responsibility classes
class UserRepository {
  create(data: UserData): User { ... }
  update(id: string, data: Partial<UserData>): User { ... }
  delete(id: string): void { ... }
}

class EmailService {
  send(to: string, content: string): void { ... }
}

class PasswordService {
  validate(password: string): ValidationResult { ... }
  hash(password: string): string { ... }
}

Detection: Class has >20 methods, >500 lines, or handles unrelated concerns.

Long Method

Functions that do too much and are hard to understand.

# BAD: Long method doing everything
def process_order(order_data):
    # Validate order (20 lines)
    if not order_data.get('items'):
        raise ValueError('No items')
    if not order_data.get('customer_id'):
        raise ValueError('No customer')
    # ... more validation

    # Calculate totals (30 lines)
    subtotal = 0
    for item in order_data['items']:
        price = get_product_price(item['product_id'])
        subtotal += price * item['quantity']
    # ... tax calculation, discounts

    # Process payment (40 lines)
    payment_result = payment_gateway.charge(...)
    # ... handle payment errors

    # Create order record (20 lines)
    order = Order.create(...)

    # Send notifications (20 lines)
    send_order_confirmation(...)
    notify_warehouse(...)

    return order

# GOOD: Composed of focused functions
def process_order(order_data):
    validate_order(order_data)
    totals = calculate_order_totals(order_data)
    payment = process_payment(order_data['customer_id'], totals)
    order = create_order_record(order_data, totals, payment)
    send_order_notifications(order)
    return order

Detection: Function >50 lines or requires scrolling to read.

Deep Nesting

Excessive indentation making code hard to follow.

// BAD: Deep nesting
function processData(data) {
  if (data) {
    if (data.items) {
      if (data.items.length > 0) {
        for (const item of data.items) {
          if (item.isValid) {
            if (item.type === 'premium') {
              if (item.price > 100) {
                // Finally do something
                processItem(item);
              }
            }
          }
        }
      }
    }
  }
}

// GOOD: Early returns and guard clauses
function processData(data) {
  if (!data?.items?.length) {
    return;
  }

  const premiumItems = data.items.filter(
    item => item.isValid && item.type === 'premium' && item.price > 100
  );

  premiumItems.forEach(processItem);
}

Detection: Indentation >4 levels deep.

Magic Numbers and Strings

Hard-coded values without explanation.

// BAD: Magic numbers
func calculateDiscount(total float64, userType int) float64 {
    if userType == 1 {
        return total * 0.15
    } else if userType == 2 {
        return total * 0.25
    }
    return total * 0.05
}

// GOOD: Named constants
const (
    UserTypeRegular  = 1
    UserTypePremium  = 2

    DiscountRegular  = 0.05
    DiscountStandard = 0.15
    DiscountPremium  = 0.25
)

func calculateDiscount(total float64, userType int) float64 {
    switch userType {
    case UserTypePremium:
        return total * DiscountPremium
    case UserTypeRegular:
        return total * DiscountStandard
    default:
        return total * DiscountRegular
    }
}

Detection: Literal numbers (except 0, 1) or repeated string literals.

Primitive Obsession

Using primitives instead of small objects.

// BAD: Primitives everywhere
function createUser(
  name: string,
  email: string,
  phone: string,
  street: string,
  city: string,
  zipCode: string,
  country: string
): User { ... }

// GOOD: Value objects
interface Address {
  street: string;
  city: string;
  zipCode: string;
  country: string;
}

interface ContactInfo {
  email: string;
  phone: string;
}

function createUser(
  name: string,
  contact: ContactInfo,
  address: Address
): User { ... }

Detection: Functions with >4 parameters of same type, or related primitives always passed together.

Logic Antipatterns

Boolean Blindness

Passing booleans that make code unreadable at call sites.

// BAD: What do these booleans mean?
user.configure(true, false, true, false)

// GOOD: Named parameters or option objects
user.configure(
    sendWelcomeEmail: true,
    requireVerification: false,
    enableNotifications: true,
    isAdmin: false
)

// Or use an options struct
struct UserConfiguration {
    var sendWelcomeEmail: Bool = true
    var requireVerification: Bool = false
    var enableNotifications: Bool = true
    var isAdmin: Bool = false
}

user.configure(UserConfiguration())

Detection: Function calls with multiple boolean literals.

Null Returns for Collections

Returning null instead of empty collections.

// BAD: Returning null
fun findUsersByRole(role: String): List<User>? {
    val users = repository.findByRole(role)
    return if (users.isEmpty()) null else users
}

// Caller must handle null
val users = findUsersByRole("admin")
if (users != null) {
    users.forEach { ... }
}

// GOOD: Return empty collection
fun findUsersByRole(role: String): List<User> {
    return repository.findByRole(role)
}

// Caller can iterate directly
findUsersByRole("admin").forEach { ... }

Detection: Functions returning nullable collections.

Stringly Typed Code

Using strings where enums or types should be used.

# BAD: String-based logic
def handle_event(event_type: str, data: dict):
    if event_type == "user_created":
        handle_user_created(data)
    elif event_type == "user_updated":
        handle_user_updated(data)
    elif event_type == "user_dleted":  # Typo won't be caught
        handle_user_deleted(data)

# GOOD: Enum-based
from enum import Enum

class EventType(Enum):
    USER_CREATED = "user_created"
    USER_UPDATED = "user_updated"
    USER_DELETED = "user_deleted"

def handle_event(event_type: EventType, data: dict):
    handlers = {
        EventType.USER_CREATED: handle_user_created,
        EventType.USER_UPDATED: handle_user_updated,
        EventType.USER_DELETED: handle_user_deleted,
    }
    handlers[event_type](data)

Detection: String comparisons for type/status/category values.

Security Antipatterns

SQL Injection

String concatenation in SQL queries.

// BAD: String concatenation
const query = `SELECT * FROM users WHERE id = ${userId}`;
db.query(query);

// BAD: String templates still vulnerable
const query = `SELECT * FROM users WHERE name = '${userName}'`;

// GOOD: Parameterized queries
const query = 'SELECT * FROM users WHERE id = $1';
db.query(query, [userId]);

// GOOD: Using ORM safely
User.findOne({ where: { id: userId } });

Detection: String concatenation or template literals with SQL keywords.

Hardcoded Credentials

Secrets in source code.

# BAD: Hardcoded secrets
API_KEY = "sk-abc123xyz789"
DATABASE_URL = "postgresql://admin:[email protected]:5432/app"

# GOOD: Environment variables
import os

API_KEY = os.environ["API_KEY"]
DATABASE_URL = os.environ["DATABASE_URL"]

# GOOD: Secrets manager
from aws_secretsmanager import get_secret

API_KEY = get_secret("api-key")

Detection: Variables named password, secret, key, token with string literals.

Unsafe Deserialization

Deserializing untrusted data without validation.

# BAD: Binary serialization from untrusted source can execute arbitrary code
# Examples: Python's binary serialization, yaml.load without SafeLoader

# GOOD: Use safe alternatives
import json

def load_data(file_path):
    with open(file_path, 'r') as f:
        return json.load(f)

# GOOD: Use SafeLoader for YAML
import yaml

with open('config.yaml') as f:
    config = yaml.safe_load(f)

Detection: Binary deserialization functions, yaml.load without safe loader, dynamic code execution on external data.

Missing Input Validation

Trusting user input without validation.

// BAD: No validation
app.post('/user', (req, res) => {
  const user = db.create({
    name: req.body.name,
    email: req.body.email,
    role: req.body.role  // User can set themselves as admin!
  });
  res.json(user);
});

// GOOD: Validate and sanitize
import { z } from 'zod';

const CreateUserSchema = z.object({
  name: z.string().min(1).max(100),
  email: z.string().email(),
  // role is NOT accepted from input
});

app.post('/user', (req, res) => {
  const validated = CreateUserSchema.parse(req.body);
  const user = db.create({
    ...validated,
    role: 'user'  // Default role, not from input
  });
  res.json(user);
});

Detection: Request body/params used directly without validation schema.

Performance Antipatterns

N+1 Query Problem

Loading related data one record at a time.

# BAD: N+1 queries
def get_orders_with_items():
    orders = Order.query.all()  # 1 query
    for order in orders:
        items = OrderItem.query.filter_by(order_id=order.id).all()  # N queries
        order.items = items
    return orders

# GOOD: Eager loading
def get_orders_with_items():
    return Order.query.options(
        joinedload(Order.items)
    ).all()  # 1 query with JOIN

# GOOD: Batch loading
def get_orders_with_items():
    orders = Order.query.all()
    order_ids = [o.id for o in orders]
    items = OrderItem.query.filter(
        OrderItem.order_id.in_(order_ids)
    ).all()  # 2 queries total
    # Group items by order_id...

Detection: Database queries inside loops.

Unbounded Collections

Loading unlimited data into memory.

// BAD: Load all records
func GetAllUsers() ([]User, error) {
    return db.Find(&[]User{})  // Could be millions
}

// GOOD: Pagination
func GetUsers(page, pageSize int) ([]User, error) {
    offset := (page - 1) * pageSize
    return db.Limit(pageSize).Offset(offset).Find(&[]User{})
}

// GOOD: Streaming for large datasets
func ProcessAllUsers(handler func(User) error) error {
    rows, err := db.Model(&User{}).Rows()
    if err != nil {
        return err
    }
    defer rows.Close()

    for rows.Next() {
        var user User
        db.ScanRows(rows, &user)
        if err := handler(user); err != nil {
            return err
        }
    }
    return nil
}

Detection: findAll(), find({}), or queries without LIMIT.

Synchronous I/O in Hot Paths

Blocking operations in request handlers.

// BAD: Sync file read on every request
app.get('/config', (req, res) => {
  const config = fs.readFileSync('./config.json');  // Blocks event loop
  res.json(JSON.parse(config));
});

// GOOD: Load once at startup
const config = JSON.parse(fs.readFileSync('./config.json'));

app.get('/config', (req, res) => {
  res.json(config);
});

// GOOD: Async with caching
let configCache = null;

app.get('/config', async (req, res) => {
  if (!configCache) {
    configCache = JSON.parse(await fs.promises.readFile('./config.json'));
  }
  res.json(configCache);
});

Detection: readFileSync, execSync, or blocking calls in request handlers.

Testing Antipatterns

Test Code Duplication

Repeating setup in every test.

// BAD: Duplicate setup
describe('UserService', () => {
  it('should create user', async () => {
    const db = await createTestDatabase();
    const userRepo = new UserRepository(db);
    const emailService = new MockEmailService();
    const service = new UserService(userRepo, emailService);

    const user = await service.create({ name: 'Test' });
    expect(user.name).toBe('Test');
  });

  it('should update user', async () => {
    const db = await createTestDatabase();  // Duplicated
    const userRepo = new UserRepository(db);  // Duplicated
    const emailService = new MockEmailService();  // Duplicated
    const service = new UserService(userRepo, emailService);  // Duplicated

    // ...
  });
});

// GOOD: Shared setup
describe('UserService', () => {
  let service: UserService;
  let db: TestDatabase;

  beforeEach(async () => {
    db = await createTestDatabase();
    const userRepo = new UserRepository(db);
    const emailService = new MockEmailService();
    service = new UserService(userRepo, emailService);
  });

  afterEach(async () => {
    await db.cleanup();
  });

  it('should create user', async () => {
    const user = await service.create({ name: 'Test' });
    expect(user.name).toBe('Test');
  });
});

Testing Implementation Instead of Behavior

Tests coupled to internal implementation.

# BAD: Testing implementation details
def test_add_item_to_cart():
    cart = ShoppingCart()
    cart.add_item(Product("Apple", 1.00))

    # Testing internal structure
    assert cart._items[0].name == "Apple"
    assert cart._total == 1.00

# GOOD: Testing behavior
def test_add_item_to_cart():
    cart = ShoppingCart()
    cart.add_item(Product("Apple", 1.00))

    # Testing public behavior
    assert cart.item_count == 1
    assert cart.total == 1.00
    assert cart.contains("Apple")

Async Antipatterns

Floating Promises

Promises without await or catch.

// BAD: Floating promise
async function saveUser(user: User) {
  db.save(user);  // Not awaited, errors lost
  logger.info('User saved');  // Logs before save completes
}

// BAD: Fire and forget in loop
for (const item of items) {
  processItem(item);  // All run in parallel, no error handling
}

// GOOD: Await the promise
async function saveUser(user: User) {
  await db.save(user);
  logger.info('User saved');
}

// GOOD: Process with proper handling
await Promise.all(items.map(item => processItem(item)));

// Or sequentially
for (const item of items) {
  await processItem(item);
}

Detection: Async function calls without await or .then().

Callback Hell

Deeply nested callbacks.

// BAD: Callback hell
getUser(userId, (err, user) => {
  if (err) return handleError(err);
  getOrders(user.id, (err, orders) => {
    if (err) return handleError(err);
    getProducts(orders[0].productIds, (err, products) => {
      if (err) return handleError(err);
      renderPage(user, orders, products, (err) => {
        if (err) return handleError(err);
        console.log('Done');
      });
    });
  });
});

// GOOD: Async/await
async function loadPage(userId) {
  try {
    const user = await getUser(userId);
    const orders = await getOrders(user.id);
    const products = await getProducts(orders[0].productIds);
    await renderPage(user, orders, products);
    console.log('Done');
  } catch (err) {
    handleError(err);
  }
}

Detection: >2 levels of callback nesting.

Async in Constructor

Async operations in constructors.

// BAD: Async in constructor
class DatabaseConnection {
  constructor(url: string) {
    this.connect(url);  // Fire-and-forget async
  }

  private async connect(url: string) {
    this.client = await createClient(url);
  }
}

// GOOD: Factory method
class DatabaseConnection {
  private constructor(private client: Client) {}

  static async create(url: string): Promise<DatabaseConnection> {
    const client = await createClient(url);
    return new DatabaseConnection(client);
  }
}

// Usage
const db = await DatabaseConnection.create(url);

Detection: async calls or .then() in constructor.