Rust + SQLite Project Knowledge Base & Code Review Guide

⚠️ CRITICAL AI CAPABILITY WARNING ⚠️

As an AI, I can make code suggestion mistakes. Especially when I have no idea about the solution, I nevertheless make a hallucinated suggestion instead of saying "I don't know". The error ratio is much higher for newer libraries because I am not trained on them.

However, I am very good at finding anti-patterns and code that smells.

AI Strengths ✅

• Excellent at detecting anti-patterns and code smells
• Strong at identifying security vulnerabilities in common patterns
• Good at suggesting refactoring for better code structure
• Reliable for well-established Rust patterns (pre-2024)
• Effective at spotting performance issues in common scenarios

AI Limitations ❌

• May hallucinate solutions for unfamiliar problems instead of admitting uncertainty
• Higher error rate for newer libraries (post-2023)
• May suggest outdated patterns for rapidly evolving frameworks
• Cannot test code execution - suggestions may not compile
• Limited understanding of project-specific context and business requirements

How to Use This Guide Safely

1. Always verify AI suggestions with actual compilation and testing
2. Be extra cautious with newer libraries like Leptos 0.6+ features
3. Trust anti-pattern detection more than solution generation
4. Cross-reference with official documentation for current APIs
5. Use AI for code review and pattern detection, not as sole source of truth

---

Reviewed by Claude.ai

---

📚 Table of Contents

1. Code Review Structure & Methodology
2. Project-Specific Anti-Patterns
3. Rust Anti-Patterns Comprehensive Guide
4. SQL & Database Anti-Patterns
5. Functional Programming Anti-Patterns
6. Architecture & Design Anti-Patterns
7. Testing Anti-Patterns
8. Security Anti-Patterns
9. Performance Anti-Patterns
10. Project-Specific Guidelines

---

Code Review Structure & Methodology

Review Process Order

Reviewed by Claude.ai

0. ⚠️ AI Limitations Reminder (Always include in reviews)

Start every review with a brief reminder:

"AI Review Notice: While I'm effective at detecting anti-patterns and code smells, please verify any specific solutions I suggest, especially for newer libraries. I may generate plausible-looking but incorrect code rather than admitting uncertainty."

1. 🏆 Excellence Recognition - Always start with what's done well
2. 🚨 Critical Issues - Security, correctness, performance blockers
3. ⚠️ Medium Priority Problems - Structural concerns, maintenance risks
4. 💡 Optional Improvements - Style, optimization suggestions
5. 📋 Pre-Merge Checklist - Essential steps before merging

Review Template Example

## Code Review by Claude.ai

### ⚠️ AI Review Notice
While I'm effective at detecting anti-patterns and code smells, please verify any specific solutions I suggest, especially for newer libraries. I may generate plausible-looking but incorrect code rather than admitting uncertainty.

### 🏆 What's Excellent
- [Specific praise for good patterns]

### 🚨 Critical Issues (Must Fix)
- [Issue with confidence level: HIGH/MEDIUM/LOW]
  - **Problem**: [Description]
  - **Confidence**: HIGH (this is a well-known anti-pattern)
  - **Suggested Fix**: [Solution - verify compilation]

### ⚠️ Medium Priority Issues
- [Issue]
  - **Confidence**: MEDIUM (pattern looks suspicious but context-dependent)
  
### 💡 Suggestions
- [Improvement]
  - **Note**: This suggestion involves newer Leptos features - please verify with current docs

### 📋 Pre-Merge Checklist
- [ ] Verify all suggested fixes compile
- [ ] Check solutions against current documentation
- [ ] Run tests after implementing changes

Multi-Prompt Review Strategy

• Comprehensive Coverage: Continue until all aspects are covered
• Depth Over Breadth: Deep insights, not superficial observations
• Progressive Detail: Start high-level, dive into specifics
• Context Preservation: Each continuation builds on previous findings
• Completion Indicator: Signal when review is complete

Pattern Spotting Algorithm

1. Scan for Signatures: Method patterns, struct design, control flow
2. Context Analysis: Identify misplaced/misused patterns
3. Semantic Evaluation: Does it do what it's supposed to?
4. Impact Assessment: What happens if unchanged?
5. Solution Generation: Clear, actionable refactors
6. Query Scanning: Match bind placeholders with struct fields

Red Flag Indicators

• Complexity spikes (high cyclomatic complexity)
• Repetition patterns (repeated blocks with tweaks)
• Inconsistent patterns (mixed paradigms)
• Performance bottlenecks (unnecessary allocations)
• Error handling gaps (missing branches)
• Bind mismatches (SQL parameters out of sync)

---

Project-Specific Anti-Patterns

1. AccountCtx Usage Anti-Pattern

Problem: AccountCtx cannot be used in any resource.rs Solution: Update new() to new(canonical_name) and solve related view issues Note: Don't praise "consistent use of AccountCtx::use_context()" - we don't want globals in server-side code

2. SELECT Clause & Struct Dependencies

Problem: Modifying SELECT statements without updating corresponding Rust structs Detection:

• Look for changes to SELECT clauses
• Check if queries map to Rust structs
• Verify struct field order matches SELECT column order

Review Template:

🔧 **Struct Dependency Check Required**
I notice you've modified the SELECT clause in `query_name.sql` but I don't see corresponding changes to the `StructName` struct.

Please verify:
- [ ] Struct updated to match new column selection
- [ ] New fields properly typed (nullable/optional as needed)
- [ ] Column order matches struct field order
- [ ] No breaking changes affecting other team members

3. Hard-Coded Dates Anti-Pattern

Problem: Hard-coded dates and years require constant maintenance Solution: Use configuration files or dynamic date calculations Example: Replace WHERE year = 2023 with dynamic date ranges

---

Rust Anti-Patterns Comprehensive Guide

Ownership & Borrowing Anti-Patterns

1. Clone Everything Anti-Pattern

// ❌ Anti-pattern: Unnecessary cloning
fn process_data(data: Vec<String>) -> Vec<String> {
    data.clone().into_iter().map(|s| s.clone().to_uppercase()).collect()
}

// ✅ Better: Use references
fn process_data(data: &[String]) -> Vec<String> {
    data.iter().map(|s| s.to_uppercase()).collect()
}

2. Fighting the Borrow Checker

// ❌ Anti-pattern: RefCell/Rc everywhere
use std::rc::Rc;
use std::cell::RefCell;

struct BadDesign {
    data: Rc<RefCell<Vec<i32>>>,
}

// ✅ Better: Work with ownership model
struct GoodDesign {
    data: Vec<i32>,
}

3. Lifetime Soup

// ❌ Anti-pattern: Excessive lifetime annotations
fn process<'a, 'b, 'c>(x: &'a str, y: &'b str, z: &'c str) -> &'a str 
where 'b: 'a, 'c: 'b {
    x
}

// ✅ Better: Simplify lifetimes
fn process(x: &str, _y: &str, _z: &str) -> &str {
    x
}

String Handling Anti-Patterns

1. Inefficient String Operations

// ❌ Anti-pattern: String concatenation in loops
let mut result = String::new();
for item in items {
    result = result + &item.to_string() + ",";
}

// ✅ Better: Use push_str or format!
let mut result = String::new();
for item in items {
    if !result.is_empty() {
        result.push(',');
    }
    result.push_str(&item.to_string());
}

2. String vs &str Confusion

// ❌ Anti-pattern: Always using String
fn process_text(text: String) -> String {
    text.to_uppercase()
}

// ✅ Better: Use &str for read-only
fn process_text(text: &str) -> String {
    text.to_uppercase()
}

Error Handling Anti-Patterns

1. Unwrap Cascades

// ❌ Anti-pattern: Chaining unwraps
let result = data.get(0).unwrap().parse::<i32>().unwrap();

// ✅ Better: Proper error handling
let result = data.get(0)
    .ok_or("No data")?
    .parse::<i32>()?;

2. Ignoring Errors

// ❌ Anti-pattern: Silent error ignoring
let _ = some_fallible_operation();

// ✅ Better: Handle or log errors
if let Err(e) = some_fallible_operation() {
    eprintln!("Operation failed: {}", e);
}

3. Panic in Libraries

// ❌ Anti-pattern: Panicking in library code
pub fn divide(a: i32, b: i32) -> i32 {
    if b == 0 {
        panic!("Division by zero!");
    }
    a / b
}

// ✅ Better: Return Result
pub fn divide(a: i32, b: i32) -> Result<i32, DivisionError> {
    if b == 0 {
        return Err(DivisionError::DivisionByZero);
    }
    Ok(a / b)
}

Collection Anti-Patterns

1. Collecting When Not Needed

// ❌ Anti-pattern: Unnecessary collection
let sum: i32 = data.iter()
    .map(|x| x * 2)
    .collect::<Vec<_>>()
    .iter()
    .sum();

// ✅ Better: Chain iterators
let sum: i32 = data.iter()
    .map(|x| x * 2)
    .sum();

2. Index-Based Iteration

// ❌ Anti-pattern: C-style loops
for i in 0..vec.len() {
    println!("{}", vec[i]);
}

// ✅ Better: Use iterators
for item in &vec {
    println!("{}", item);
}

Async/Await Anti-Patterns

1. Blocking in Async Context

// ❌ Anti-pattern: Blocking operations in async
async fn bad_async() -> Result<String, Error> {
    let data = std::fs::read_to_string("file.txt")?; // Blocking!
    Ok(data)
}

// ✅ Better: Use async alternatives
async fn good_async() -> Result<String, Error> {
    tokio::fs::read_to_string("file.txt").await
}

2. Sequential Async Operations

// ❌ Anti-pattern: Sequential when could be concurrent
async fn sequential() -> (Result<A, Error>, Result<B, Error>) {
    let a = fetch_a().await;
    let b = fetch_b().await;
    (a, b)
}

// ✅ Better: Concurrent execution
async fn concurrent() -> (Result<A, Error>, Result<B, Error>) {
    tokio::join!(fetch_a(), fetch_b())
}

Memory Management Anti-Patterns

1. Arc Overuse

// ❌ Anti-pattern: Arc for everything
struct OverArc {
    name: Arc<String>,
    age: Arc<i32>,
}

// ✅ Better: Arc only when needed
struct BetterDesign {
    name: String,
    age: i32,
}

2. Premature Optimization with Unsafe

// ❌ Anti-pattern: Unnecessary unsafe
unsafe {
    let ptr = data.as_ptr();
    *ptr.add(index)
}

// ✅ Better: Safe alternatives
data.get(index).copied().unwrap_or_default()

3. Leaking Memory with Forgotten Drops

// ❌ Anti-pattern: Not handling resources properly
fn process_file() -> Result<String, Error> {
    let file = File::create("temp.txt")?;
    // File handle might leak if error occurs before Drop
    let data = risky_operation()?;
    write!(file, "{}", data)?;
    Ok(data)
}

// ✅ Better: Use RAII patterns or explicit resource management
fn process_file() -> Result<String, Error> {
    {
        let mut file = File::create("temp.txt")?;
        let data = risky_operation()?;
        write!(file, "{}", data)?;
        // File automatically closed when going out of scope
    } // Explicit scope ensures cleanup
    Ok(data)
}

Concurrency Anti-Patterns

1. Shared Mutable State Without Synchronization

// ❌ Anti-pattern: Race conditions with unsafe static
static mut COUNTER: i32 = 0;

unsafe fn increment() {
    COUNTER += 1; // Race condition!
}

// ✅ Better: Use atomic types
use std::sync::atomic::{AtomicI32, Ordering};
static COUNTER: AtomicI32 = AtomicI32::new(0);

fn increment() {
    COUNTER.fetch_add(1, Ordering::SeqCst);
}

// ✅ Or use Mutex for complex state
use std::sync::Mutex;
static COUNTER: Mutex<i32> = Mutex::new(0);

fn increment() {
    let mut counter = COUNTER.lock().unwrap();
    *counter += 1;
}

---

SQL & Database Anti-Patterns

Query Construction Anti-Patterns

1. SQL Injection Vulnerability

// ❌ Anti-pattern: String concatenation
let query = format!("SELECT * FROM users WHERE name = '{}'", user_input);

// ✅ Better: Parameterized queries
let user = sqlx::query_as::<_, User>("SELECT * FROM users WHERE name = ?")
    .bind(user_input)
    .fetch_one(pool)
    .await?;

2. N+1 Query Problem

// ❌ Anti-pattern: Query in loop
for user in users {
    let orders = sqlx::query_as::<_, Order>(
        "SELECT * FROM orders WHERE user_id = ?"
    )
    .bind(user.id)
    .fetch_all(pool).await?;
}

// ✅ Better: Single query with JOIN
let query = r#"
    SELECT u.*, o.* 
    FROM users u 
    LEFT JOIN orders o ON u.id = o.user_id
"#;

3. SELECT * Anti-Pattern

-- ❌ Anti-pattern: Fetching all columns
SELECT * FROM users WHERE id = ?

-- ✅ Better: Specify needed columns
SELECT id, name, email FROM users WHERE id = ?

SQL Query Writing Anti-Patterns

1. Implicit Joins with WHERE Clauses

-- ❌ Anti-pattern: Old-style implicit joins
SELECT * FROM users, orders 
WHERE users.id = orders.user_id

-- ✅ Better: Explicit JOIN syntax
SELECT * FROM users 
JOIN orders ON users.id = orders.user_id

2. Wrong JOIN Types

-- ❌ Anti-pattern: Using INNER JOIN when you need LEFT JOIN
SELECT u.*, o.* 
FROM users u 
INNER JOIN orders o ON u.id = o.user_id
-- Missing users without orders!

-- ✅ Better: Use appropriate JOIN type
SELECT u.*, o.* 
FROM users u 
LEFT JOIN orders o ON u.id = o.user_id
-- Includes all users, even without orders

3. Joining on Non-Indexed Columns

-- ❌ Anti-pattern: Repeatedly joining on unindexed columns
SELECT * FROM orders o
JOIN users u ON u.email = o.user_email -- email not indexed!
JOIN products p ON p.sku = o.product_sku -- sku not indexed!

-- ✅ Better: Join on indexed columns (typically IDs)
SELECT * FROM orders o
JOIN users u ON u.id = o.user_id
JOIN products p ON p.id = o.product_id

WHERE Clause Anti-Patterns

1. Functions in WHERE Predicates

-- ❌ Anti-pattern: Function prevents index usage
WHERE YEAR(created_date) = 2023
WHERE UPPER(email) = '[email protected]'
WHERE SUBSTRING(phone, 1, 3) = '555'

-- ✅ Better: Range comparison or exact match
WHERE created_date >= '2023-01-01' AND created_date < '2024-01-01'
WHERE email = '[email protected]' -- Store lowercase
WHERE phone LIKE '555%' -- Can use index for prefix

2. OR with Different Columns

-- ❌ Anti-pattern: OR across columns can't use indexes effectively
WHERE status = 'active' OR created_date > '2023-01-01'

-- ✅ Better: Use UNION for better index usage
SELECT * FROM users WHERE status = 'active'
UNION
SELECT * FROM users WHERE created_date > '2023-01-01'

3. NOT IN with NULL Values

-- ❌ Anti-pattern: NULL in NOT IN returns no rows
WHERE id NOT IN (1, 2, NULL) -- Returns nothing!

-- ✅ Better: Use NOT EXISTS or handle NULLs
WHERE NOT EXISTS (
    SELECT 1 FROM excluded 
    WHERE excluded.id = users.id
)

-- Or filter NULLs explicitly
WHERE id NOT IN (
    SELECT id FROM excluded WHERE id IS NOT NULL
)

4. Comparing Different Data Types

-- ❌ Anti-pattern: Type confusion
WHERE user_id = '123' -- user_id is INTEGER
WHERE amount > '100.00' -- amount is REAL

-- ✅ Better: Use correct types
WHERE user_id = 123
WHERE amount > 100.00

SELECT Clause Anti-Patterns

1. DISTINCT as a Crutch

-- ❌ Anti-pattern: Using DISTINCT to fix bad JOINs
SELECT DISTINCT u.name 
FROM users u
JOIN orders o ON u.id = o.user_id
-- DISTINCT hides the real problem

-- ✅ Better: Fix the JOIN logic
SELECT u.name 
FROM users u
WHERE EXISTS (
    SELECT 1 FROM orders o WHERE o.user_id = u.id
)

2. Selecting Large TEXT/BLOB Columns Unnecessarily

-- ❌ Anti-pattern: Always selecting large columns
SELECT id, name, email, biography, profile_image_data 
FROM users 
WHERE active = true

-- ✅ Better: Only select large columns when needed
-- For list view:
SELECT id, name, email FROM users WHERE active = true

-- For detail view:
SELECT * FROM users WHERE id = ?

3. ORDER BY without LIMIT

-- ❌ Anti-pattern: Sorting entire large table
SELECT * FROM logs ORDER BY created_at DESC

-- ✅ Better: Always paginate sorted results
SELECT * FROM logs 
ORDER BY created_at DESC 
LIMIT 100 OFFSET 0

Subquery Anti-Patterns

1. Correlated Subqueries in SELECT

-- ❌ Anti-pattern: Subquery executes for each row
SELECT 
    u.name,
    (SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id) as order_count
FROM users u

-- ✅ Better: Use JOIN with GROUP BY
SELECT 
    u.name,
    COUNT(o.id) as order_count
FROM users u
LEFT JOIN orders o ON u.id = o.user_id
GROUP BY u.id, u.name

2. EXISTS vs IN Misuse

-- ❌ Anti-pattern: IN with large subquery
WHERE user_id IN (
    SELECT user_id FROM orders WHERE total > 1000
)

-- ✅ Better: Use EXISTS for existence checks
WHERE EXISTS (
    SELECT 1 FROM orders o 
    WHERE o.user_id = users.id AND o.total > 1000
)

3. Nested Subqueries Instead of CTEs

-- ❌ Anti-pattern: Hard to read nested subqueries
SELECT * FROM (
    SELECT * FROM (
        SELECT * FROM users WHERE active = true
    ) WHERE created_date > '2023-01-01'
) WHERE email LIKE '%@company.com'

-- ✅ Better: Use CTEs for clarity
WITH active_users AS (
    SELECT * FROM users WHERE active = true
),
recent_users AS (
    SELECT * FROM active_users WHERE created_date > '2023-01-01'
)
SELECT * FROM recent_users WHERE email LIKE '%@company.com'

Aggregation Anti-Patterns

1. Mixing Aggregated and Non-Aggregated Columns

-- ❌ Anti-pattern: Missing GROUP BY
SELECT name, COUNT(*) FROM users
-- SQLite might return random name!

-- ✅ Better: Proper GROUP BY
SELECT name, COUNT(*) as count 
FROM users 
GROUP BY name

2. HAVING Instead of WHERE

-- ❌ Anti-pattern: Filtering with HAVING
SELECT user_id, COUNT(*) as order_count
FROM orders
GROUP BY user_id
HAVING user_id > 100 -- Should be WHERE

-- ✅ Better: Filter before grouping
SELECT user_id, COUNT(*) as order_count
FROM orders
WHERE user_id > 100 -- Filter first
GROUP BY user_id
HAVING COUNT(*) > 5 -- HAVING for aggregates only

3. Inefficient Window Functions

-- ❌ Anti-pattern: Window function without partition
SELECT 
    *,
    ROW_NUMBER() OVER (ORDER BY created_date) as rn
FROM large_table

-- ✅ Better: Partition when possible
SELECT 
    *,
    ROW_NUMBER() OVER (
        PARTITION BY category 
        ORDER BY created_date
    ) as rn
FROM large_table

CASE Statement Anti-Patterns

1. Complex Nested CASE

-- ❌ Anti-pattern: Deeply nested CASE
SELECT 
    CASE 
        WHEN status = 'active' THEN
            CASE 
                WHEN level > 5 THEN
                    CASE 
                        WHEN premium THEN 'VIP'
                        ELSE 'Regular'
                    END
                ELSE 'Basic'
            END
        ELSE 'Inactive'
    END as user_type

-- ✅ Better: Simplify or use lookup table
SELECT 
    CASE 
        WHEN status != 'active' THEN 'Inactive'
        WHEN premium AND level > 5 THEN 'VIP'
        WHEN level > 5 THEN 'Regular'
        ELSE 'Basic'
    END as user_type

2. CASE for Simple Boolean Logic

-- ❌ Anti-pattern: Unnecessary CASE
SELECT 
    CASE WHEN status = 'active' THEN 1 ELSE 0 END as is_active

-- ✅ Better: Direct boolean expression
SELECT 
    (status = 'active') as is_active

Data Type Anti-Patterns

1. String Date Comparisons

-- ❌ Anti-pattern: String comparison for dates
WHERE date_column > '2023-1-1' -- Wrong format!
WHERE date_column BETWEEN '01/01/2023' AND '12/31/2023'

-- ✅ Better: Use ISO format or date functions
WHERE date_column > '2023-01-01'
WHERE date_column >= '2023-01-01' AND date_column < '2024-01-01'

2. Collation Issues

-- ❌ Anti-pattern: Unexpected case sensitivity
WHERE email = '[email protected]' -- May not match '[email protected]'

-- ✅ Better: Be explicit about collation
WHERE email = '[email protected]' COLLATE NOCASE
-- Or store normalized (lowercase) values

Logic Anti-Patterns

1. UNION vs UNION ALL

-- ❌ Anti-pattern: UNION when duplicates impossible
SELECT id, name FROM customers
UNION -- Expensive duplicate removal
SELECT id, name FROM suppliers

-- ✅ Better: Use UNION ALL when no duplicates
SELECT id, name FROM customers
UNION ALL -- No duplicate check
SELECT id, name FROM suppliers

2. Complex WHERE Without Parentheses

-- ❌ Anti-pattern: Ambiguous logic
WHERE status = 'active' AND level > 5 OR premium = true
-- Is it (A AND B) OR C or A AND (B OR C)?

-- ✅ Better: Explicit parentheses
WHERE status = 'active' AND (level > 5 OR premium = true)

3. Row-by-Row Processing

// ❌ Anti-pattern: Processing in application
let users = get_all_users().await?;
for user in users {
    if user.age > 18 {
        update_user_status(user.id, "adult").await?;
    }
}

// ✅ Better: Set-based operation
sqlx::query("UPDATE users SET status = 'adult' WHERE age > 18")
    .execute(pool)
    .await?;

NULL Handling Anti-Patterns

1. NULL in Comparisons

-- ❌ Anti-pattern: Comparing with NULL
WHERE column = NULL -- Always false!
WHERE column != NULL -- Always false!

-- ✅ Better: Use IS NULL/IS NOT NULL
WHERE column IS NULL
WHERE column IS NOT NULL

2. NULL in Math Operations

-- ❌ Anti-pattern: Math with potential NULLs
SELECT price * quantity as total -- NULL if either is NULL

-- ✅ Better: Handle NULLs explicitly
SELECT 
    COALESCE(price, 0) * COALESCE(quantity, 0) as total
-- Or
SELECT 
    IFNULL(price, 0) * IFNULL(quantity, 0) as total

Index Usage Anti-Patterns

1. Leading Wildcard LIKE

-- ❌ Anti-pattern: Can't use index
WHERE name LIKE '%smith' -- Leading wildcard
WHERE name LIKE '%john%' -- Both sides

-- ✅ Better: Prefix search or full-text
WHERE name LIKE 'smith%' -- Can use index
-- Or use FTS5 for full-text search

2. Negative Conditions

-- ❌ Anti-pattern: Can't use index effectively
WHERE status != 'deleted'
WHERE id NOT IN (1, 2, 3)

-- ✅ Better: Positive conditions
WHERE status IN ('active', 'pending', 'archived')
WHERE id > 3 OR id < 1

Database Connection Anti-Patterns

1. Connection Per Operation

// ❌ Anti-pattern: New connection each time
async fn bad_connection_usage() -> Result<User, Error> {
    let pool = SqlitePool::connect("sqlite:database.db").await?;
    let user = get_user(&pool, 1).await?;
    pool.close().await;
    Ok(user)
}

// ✅ Better: Reuse connection pool
struct Database {
    pool: SqlitePool,
}

impl Database {
    async fn get_user(&self, id: i64) -> Result<User, Error> {
        sqlx::query_as::<_, User>("SELECT * FROM users WHERE id = ?")
            .bind(id)
            .fetch_one(&self.pool)
            .await
    }
}

2. Transaction Misuse

// ❌ Anti-pattern: No transaction for related operations
async fn transfer_funds(pool: &SqlitePool, from: i64, to: i64, amount: f64) {
    sqlx::query("UPDATE accounts SET balance = balance - ? WHERE id = ?")
        .bind(amount).bind(from).execute(pool).await?;
    
    sqlx::query("UPDATE accounts SET balance = balance + ? WHERE id = ?")
        .bind(amount).bind(to).execute(pool).await?;
}

// ✅ Better: Use transactions
async fn transfer_funds_safe(pool: &SqlitePool, from: i64, to: i64, amount: f64) {
    let mut tx = pool.begin().await?;
    
    sqlx::query("UPDATE accounts SET balance = balance - ? WHERE id = ?")
        .bind(amount).bind(from).execute(&mut *tx).await?;
    
    sqlx::query("UPDATE accounts SET balance = balance + ? WHERE id = ?")
        .bind(amount).bind(to).execute(&mut *tx).await?;
    
    tx.commit().await?;
}

---

Functional Programming Anti-Patterns

State Management Anti-Patterns

1. Mutation-Heavy Code

// ❌ Anti-pattern: Excessive mutation
let mut results = Vec::new();
for item in items {
    if item > 10 {
        results.push(item * 2);
    }
}

// ✅ Better: Functional transformation
let results: Vec<_> = items.iter()
    .filter(|&&item| item > 10)
    .map(|&item| item * 2)
    .collect();

2. Imperative Disguise

// ❌ Anti-pattern: Imperative style in functional context
fn process_orders_imperative(orders: Vec<Order>) -> Vec<ProcessedOrder> {
    let mut processed = Vec::new();
    for order in orders {
        if order.amount > 100.0 {
            let mut order = order;
            order.priority = Priority::High;
            processed.push(ProcessedOrder::from(order));
        }
    }
    processed
}

// ✅ Better: Functional approach
fn process_orders_functional(orders: Vec<Order>) -> Vec<ProcessedOrder> {
    orders.into_iter()
        .filter(|order| order.amount > 100.0)
        .map(|order| Order { priority: Priority::High, ..order })
        .map(ProcessedOrder::from)
        .collect()
}

3. Manual Accumulation

// ❌ Anti-pattern: Manual state tracking
let mut sum = 0;
for item in items {
    sum += item;
}

// ✅ Better: Use fold/reduce
let sum = items.iter().sum();
// Or for complex operations:
let result = items.iter().fold(0, |acc, item| acc + item);

4. Stateful Processing Loops

// ❌ Anti-pattern: Mutable state in loops
let mut state = InitialState::new();
for event in events {
    match event {
        Event::A => state.handle_a(),
        Event::B => state.handle_b(),
    }
}

// ✅ Better: Functional fold
let final_state = events.iter().fold(InitialState::new(), |state, event| {
    match event {
        Event::A => state.with_a_handled(),
        Event::B => state.with_b_handled(),
    }
});

Monad and Combinator Anti-Patterns

1. Nested Map Hell

// ❌ Anti-pattern: Deeply nested maps
let result = option1.map(|x| {
    option2.map(|y| {
        option3.map(|z| {
            x + y + z
        })
    })
}); // Type: Option<Option<Option<i32>>>

// ✅ Better: Use and_then (flat_map)
let result = option1.and_then(|x| {
    option2.and_then(|y| {
        option3.map(|z| x + y + z)
    })
}); // Type: Option<i32>

// ✅ Even better: Use the ? operator
let result = (|| {
    let x = option1?;
    let y = option2?;
    let z = option3?;
    Some(x + y + z)
})();

2. Reinventing Standard Combinators

// ❌ Anti-pattern: Manual implementation of standard patterns
fn my_filter<T>(items: Vec<T>, predicate: impl Fn(&T) -> bool) -> Vec<T> {
    let mut result = Vec::new();
    for item in items {
        if predicate(&item) {
            result.push(item);
        }
    }
    result
}

// ✅ Better: Use standard library combinators
let filtered: Vec<_> = items.into_iter()
    .filter(predicate)
    .collect();

3. Overusing unwrap_or with Side Effects

// ❌ Anti-pattern: Side effects in unwrap_or
let value = expensive_computation()
    .unwrap_or(default_value()); // default_value() always called!

// ✅ Better: Use unwrap_or_else for lazy evaluation
let value = expensive_computation()
    .unwrap_or_else(|| default_value()); // Only called if None

Higher-Order Function Anti-Patterns

1. Callback Pyramid of Doom

// ❌ Anti-pattern: Deeply nested callbacks
fn process(
    data: Data,
    callback1: impl Fn(Data) -> Data,
    callback2: impl Fn(Data) -> Data,
    callback3: impl Fn(Data) -> Data,
) -> Data {
    callback3(callback2(callback1(data)))
}

// ✅ Better: Use composition
fn compose<A, B, C>(
    f: impl Fn(A) -> B,
    g: impl Fn(B) -> C,
) -> impl Fn(A) -> C {
    move |x| g(f(x))
}

// Or use a pipeline pattern
let result = data
    .pipe(callback1)
    .pipe(callback2)
    .pipe(callback3);

2. Partial Application Confusion

// ❌ Anti-pattern: Confusing partial application
fn add(x: i32) -> impl Fn(i32) -> i32 {
    move |y| x + y
}

let add5 = add(5);
let result = add5(3); // Works but unintuitive

// ✅ Better: Clear curry pattern or use closures directly
fn curry_add(x: i32, y: i32) -> i32 {
    x + y
}

let add5 = |y| curry_add(5, y);
// Or use partial application crate

Lazy Evaluation Anti-Patterns

1. Eager Collection in Chain

// ❌ Anti-pattern: Collecting too early breaks laziness
let result = data.iter()
    .map(expensive_operation)
    .collect::<Vec<_>>() // Processes all items!
    .into_iter()
    .take(5)
    .collect();

// ✅ Better: Keep it lazy
let result = data.iter()
    .map(expensive_operation)
    .take(5) // Only processes 5 items
    .collect();

2. Side Effects in Lazy Operations

// ❌ Anti-pattern: Side effects in map
let iter = data.iter().map(|x| {
    println!("Processing: {}", x); // Side effect!
    x * 2
});
// Side effects only happen when consumed - surprising!

// ✅ Better: Use inspect for debugging, separate side effects
let iter = data.iter()
    .inspect(|x| println!("Processing: {}", x))
    .map(|x| x * 2);

// Or process eagerly if side effects are needed
let results: Vec<_> = data.iter()
    .map(|x| {
        let result = x * 2;
        println!("Processed {} -> {}", x, result);
        result
    })
    .collect();

Recursion Anti-Patterns

1. Stack-Blowing Recursion

// ❌ Anti-pattern: Non-tail recursive
fn factorial(n: u64) -> u64 {
    if n <= 1 {
        1
    } else {
        n * factorial(n - 1) // Stack grows with n
    }
}

// ✅ Better: Tail recursive with accumulator
fn factorial_tail(n: u64, acc: u64) -> u64 {
    if n <= 1 {
        acc
    } else {
        factorial_tail(n - 1, n * acc)
    }
}

// ✅ Even better: Use iteration for Rust
fn factorial_iter(n: u64) -> u64 {
    (1..=n).product()
}

2. Mutual Recursion Without Trampolining

// ❌ Anti-pattern: Direct mutual recursion
fn is_even(n: u32) -> bool {
    if n == 0 {
        true
    } else {
        is_odd(n - 1) // Stack overflow for large n
    }
}

fn is_odd(n: u32) -> bool {
    if n == 0 {
        false
    } else {
        is_even(n - 1)
    }
}

// ✅ Better: Use iteration or trampolining
fn is_even_safe(n: u32) -> bool {
    n % 2 == 0
}

Functional Purity Anti-Patterns

1. Hidden Dependencies

// ❌ Anti-pattern: Function depends on external state
static CONFIG: Lazy<Config> = Lazy::new(Config::load);

fn calculate_price(amount: f64) -> f64 {
    amount * CONFIG.tax_rate // Hidden dependency!
}

// ✅ Better: Make dependencies explicit
fn calculate_price(amount: f64, tax_rate: f64) -> f64 {
    amount * tax_rate
}

// Or use dependency injection
struct PriceCalculator {
    tax_rate: f64,
}

impl PriceCalculator {
    fn calculate(&self, amount: f64) -> f64 {
        amount * self.tax_rate
    }
}

2. Time-Dependent Functions

// ❌ Anti-pattern: Function behavior changes with time
fn generate_id() -> String {
    format!("id_{}", SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs())
}

// ✅ Better: Make time an explicit parameter
fn generate_id(timestamp: SystemTime) -> String {
    format!("id_{}", timestamp.duration_since(UNIX_EPOCH).unwrap().as_secs())
}

// Or use a Clock trait for testing
trait Clock {
    fn now(&self) -> SystemTime;
}

fn generate_id_with_clock(clock: &impl Clock) -> String {
    format!("id_{}", clock.now().duration_since(UNIX_EPOCH).unwrap().as_secs())
}

Lens and Optics Anti-Patterns

1. Manual Nested Updates

// ❌ Anti-pattern: Manually updating nested structures
#[derive(Clone)]
struct Address { street: String, city: String }
#[derive(Clone)]
struct Person { name: String, address: Address }
#[derive(Clone)]
struct Company { ceo: Person }

fn update_ceo_city(company: Company, new_city: String) -> Company {
    Company {
        ceo: Person {
            address: Address {
                city: new_city,
                ..company.ceo.address
            },
            ..company.ceo
        }
    }
}

// ✅ Better: Use lens pattern or libraries
use lens_rs::*;

#[derive(Lens, Clone)]
struct Address { street: String, city: String }

#[derive(Lens, Clone)]
struct Person { name: String, address: Address }

#[derive(Lens, Clone)]
struct Company { ceo: Person }

fn update_ceo_city(company: Company, new_city: String) -> Company {
    company.set(Company::ceo.address.city, new_city)
}

Function Composition Anti-Patterns

1. Point-ful Style When Pointless Would Be Clearer

// ❌ Anti-pattern: Unnecessary lambda wrapping
let doubled = numbers.iter().map(|x| double(x));
let parsed = strings.iter().map(|s| s.parse::<i32>());

// ✅ Better: Point-free style when clearer
let doubled = numbers.iter().map(double);
let parsed = strings.iter().map(str::parse::<i32>);

2. Over-Abstraction

// ❌ Anti-pattern: Too abstract to understand
fn lift2<A, B, C, F, G>(f: F) -> impl Fn(G, G) -> G
where
    F: Fn(A, B) -> C,
    G: Fn() -> A + Fn() -> B + Fn() -> C,
{
    // Overly complex implementation
}

// ✅ Better: Balance abstraction with clarity
fn combine_results<T, E>(
    result1: Result<T, E>,
    result2: Result<T, E>,
    combiner: impl Fn(T, T) -> T,
) -> Result<T, E> {
    Ok(combiner(result1?, result2?))
}

Conditional Logic Anti-Patterns

1. Nested If-Else Chains

// ❌ Anti-pattern: Deep nesting
let mut result = String::new();
if condition1 {
    if condition2 {
        result = "case1".to_string();
    } else {
        result = "case2".to_string();
    }
} else {
    result = "case3".to_string();
}

// ✅ Better: Pattern matching
let result = match (condition1, condition2) {
    (true, true) => "case1",
    (true, false) => "case2",
    (false, _) => "case3",
}.to_string();

2. Boolean Blindness

// ❌ Anti-pattern: Using booleans for multiple states
fn process_user(is_admin: bool, is_active: bool, is_verified: bool) {
    if is_admin && is_active && is_verified {
        // Process admin
    } else if !is_admin && is_active && is_verified {
        // Process regular user
    }
    // Many more conditions...
}

// ✅ Better: Use enums for clear state representation
enum UserStatus {
    Admin { active: bool, verified: bool },
    Regular { active: bool, verified: bool },
    Guest,
}

fn process_user(status: UserStatus) {
    match status {
        UserStatus::Admin { active: true, verified: true } => {
            // Process active verified admin
        }
        UserStatus::Regular { active: true, verified: true } => {
            // Process active verified regular user
        }
        _ => {
            // Handle other cases
        }
    }
}

3. Early Returns with Mutable Flags

// ❌ Anti-pattern: Flag-based control flow
let mut found = false;
let mut result = None;
for item in items {
    if predicate(item) {
        result = Some(item);
        found = true;
        break;
    }
}

// ✅ Better: Iterator methods
let result = items.iter().find(|&item| predicate(item));

Error Composition Anti-Patterns

1. Nested Match Statements

// ❌ Anti-pattern: Deeply nested error handling
fn process_request(id: u64) -> Result<Response, Error> {
    match get_user(id) {
        Ok(user) => {
            match validate_user(&user) {
                Ok(()) => {
                    match get_orders(&user) {
                        Ok(orders) => Ok(Response::new(orders)),
                        Err(e) => Err(e),
                    }
                }
                Err(e) => Err(e),
            }
        }
        Err(e) => Err(e),
    }
}

// ✅ Better: Use ? operator
fn process_request_functional(id: u64) -> Result<Response, Error> {
    let user = get_user(id)?;
    validate_user(&user)?;
    let orders = get_orders(&user)?;
    Ok(Response::new(orders))
}

2. Manual Result Chaining

// ❌ Anti-pattern: Explicit result propagation
let mut final_result = Err("failed");
match step1() {
    Ok(val1) => {
        match step2(val1) {
            Ok(val2) => final_result = Ok(val2),
            Err(e) => final_result = Err(e),
        }
    }
    Err(e) => final_result = Err(e),
}

// ✅ Better: Functional chaining
let final_result = step1().and_then(step2);

3. Explicit Error Checking with Mutations

// ❌ Anti-pattern: Manual error accumulation
let mut success = true;
let mut results = Vec::new();
for item in items {
    match process(item) {
        Ok(val) => results.push(val),
        Err(_) => {
            success = false;
            break;
        }
    }
}

// ✅ Better: Collect into Result
let results: Result<Vec<_>, _> = items.iter()
    .map(|item| process(item))
    .collect();

4. Side Effects in Pure Functions

// ❌ Anti-pattern: Hidden side effects
fn calculate_tax(amount: f64) -> f64 {
    println!("Calculating tax for ${}", amount); // Side effect!
    amount * 0.08
}

// ✅ Better: Pure function
fn calculate_tax_pure(amount: f64) -> f64 {
    amount * 0.08
}

fn calculate_and_log_tax(amount: f64) -> f64 {
    let tax = calculate_tax_pure(amount);
    println!("Tax: ${}", tax);
    tax
}

Option/Result Handling Anti-Patterns

1. Nested Option Unwrapping

// ❌ Anti-pattern: Manual option handling
let mut final_result = None;
if let Some(a) = opt_a {
    if let Some(b) = opt_b {
        final_result = Some(a + b);
    }
}

// ✅ Better: Functional combinators
let final_result = opt_a.zip(opt_b).map(|(a, b)| a + b);

// Or with more complex operations:
let final_result = opt_a.and_then(|a| 
    opt_b.map(|b| a + b)
);

2. Manual None Propagation

// ❌ Anti-pattern: Explicit None checks
fn get_user_email(id: u64) -> Option<String> {
    let user = get_user(id);
    if user.is_none() {
        return None;
    }
    let user = user.unwrap();
    
    let profile = get_profile(&user);
    if profile.is_none() {
        return None;
    }
    let profile = profile.unwrap();
    
    Some(profile.email)
}

// ✅ Better: Use ? operator or and_then
fn get_user_email(id: u64) -> Option<String> {
    get_user(id)?
        .get_profile()?
        .email
}

// Or with and_then:
fn get_user_email(id: u64) -> Option<String> {
    get_user(id)
        .and_then(|user| get_profile(&user))
        .map(|profile| profile.email)
}

Collection Transformation Anti-Patterns

1. Index-Based Iteration

// ❌ Anti-pattern: C-style for loops
let mut transformed = Vec::new();
for i in 0..items.len() {
    transformed.push(transform(items[i]));
}

// ✅ Better: Iterator map
let transformed: Vec<_> = items.iter()
    .map(|item| transform(item))
    .collect();

2. Manual Grouping

// ❌ Anti-pattern: Imperative grouping
let mut groups = std::collections::HashMap::new();
for item in items {
    let key = item.category();
    if !groups.contains_key(&key) {
        groups.insert(key, Vec::new());
    }
    groups.get_mut(&key).unwrap().push(item);
}

// ✅ Better: Functional grouping
use itertools::Itertools;
let groups = items.into_iter()
    .into_group_map_by(|item| item.category());

// Or without itertools:
let groups = items.into_iter()
    .fold(HashMap::new(), |mut acc, item| {
        acc.entry(item.category())
           .or_insert_with(Vec::new)
           .push(item);
        acc
    });

3. Manual Filtering and Mapping

// ❌ Anti-pattern: Separate loops for filter and map
let mut filtered = Vec::new();
for item in items {
    if item.is_valid() {
        filtered.push(item);
    }
}

let mut mapped = Vec::new();
for item in filtered {
    mapped.push(item.transform());
}

// ✅ Better: Chain iterator adaptors
let result: Vec<_> = items.into_iter()
    .filter(|item| item.is_valid())
    .map(|item| item.transform())
    .collect();

Resource Management Anti-Patterns

1. Manual Resource Cleanup

// ❌ Anti-pattern: Explicit drop
let mut file = File::open("data.txt")?;
let mut contents = String::new();
file.read_to_string(&mut contents)?;
drop(file); // Manual cleanup

// ✅ Better: RAII and functional APIs
let contents = std::fs::read_to_string("data.txt")?;

2. Imperative File Processing

// ❌ Anti-pattern: Manual buffer management
let file = File::open("data.txt")?;
let mut reader = BufReader::new(file);
let mut lines = Vec::new();
let mut line = String::new();

loop {
    let bytes = reader.read_line(&mut line)?;
    if bytes == 0 { break; }
    lines.push(line.clone());
    line.clear();
}

// ✅ Better: Iterator-based processing
let lines: Result<Vec<String>, _> = BufReader::new(File::open("data.txt")?)
    .lines()
    .collect();

---

Design Anti-Patterns

God Object Anti-Pattern

// ❌ Anti-pattern: Monolithic structs doing everything
struct Application {
    database: Database,
    web_server: WebServer,
    cache: Cache,
    logger: Logger,
    config: Config,
    auth_service: AuthService,
    email_service: EmailService,
    payment_processor: PaymentProcessor,
    // ... many more fields making this a "God Object"
}

impl Application {
    // Hundreds of methods handling everything
    fn authenticate_user(&self) { /* ... */ }
    fn send_email(&self) { /* ... */ }
    fn process_payment(&self) { /* ... */ }
    fn query_database(&self) { /* ... */ }
    // ... 50+ more methods
}

// ✅ Better: Separate concerns with focused structs
struct Database { /* ... */ }
struct WebServer { /* ... */ }

struct Application {
    db: Database,
    server: WebServer,
}

// Even better: Use dependency injection
struct UserService {
    db: Arc<Database>,
}

struct PaymentService {
    db: Arc<Database>,
    payment_gateway: PaymentGateway,
}

Overusing Generics Anti-Pattern

// ❌ Anti-pattern: Unnecessary generic complexity
fn process<T, U, V>(input: T) -> Result<U, V>
where
    T: Clone + Debug + Send + Sync + PartialEq + Ord,
    U: Default + Clone + FromStr,
    V: Error + Send + Sync + 'static,
{
    // Complex generic constraints for simple operations
    // Makes the function hard to use and understand
}

// ✅ Better: Use generics only when truly needed
fn process_string(input: &str) -> Result<String, ProcessError> {
    // Simple, clear function signature
    // Easy to understand and use
}

// ✅ Good use of generics: When actually needed
fn find_max<T: Ord>(items: &[T]) -> Option<&T> {
    items.iter().max()
}

Premature Abstraction Anti-Pattern

// ❌ Anti-pattern: Over-engineering for hypothetical future needs
trait DataProcessor {
    type Input;
    type Output;
    type Error;
    
    fn preprocess(&self, input: Self::Input) -> Result<Self::Input, Self::Error>;
    fn process(&self, input: Self::Input) -> Result<Self::Output, Self::Error>;
    fn postprocess(&self, output: Self::Output) -> Result<Self::Output, Self::Error>;
}

struct SimpleCalculator;
impl DataProcessor for SimpleCalculator {
    // 50 lines of boilerplate for a simple addition
}

// ✅ Better: Start simple, refactor when needed
fn calculate(a: i32, b: i32) -> i32 {
    a + b
}

1. God Module

// ❌ Anti-pattern: Everything in one module
pub mod app {
    pub struct User { /* ... */ }
    pub struct Order { /* ... */ }
    pub struct Payment { /* ... */ }
    
    pub async fn get_user() { /* ... */ }
    pub async fn create_order() { /* ... */ }
    pub async fn process_payment() { /* ... */ }
}

// ✅ Better: Domain-driven organization
pub mod domain {
    pub mod user {
        pub struct User { /* ... */ }
        pub struct UserId(pub i64);
    }
    
    pub mod order {
        use super::user::UserId;
        pub struct Order { /* ... */ }
    }
}

pub mod infrastructure {
    pub mod database {
        use crate::domain::user::User;
        pub async fn get_user(pool: &SqlitePool, id: i64) -> Result<User, Error> { /* ... */ }
    }
}

2. Circular Dependencies

// ❌ Anti-pattern: Modules depend on each other
pub mod user {
    use crate::order::Order;
    pub struct User {
        pub orders: Vec<Order>, // Circular!
    }
}

pub mod order {
    use crate::user::User;
    pub struct Order {
        pub user: User, // Circular!
    }
}

// ✅ Better: Use IDs for references
pub mod user {
    #[derive(Clone)]
    pub struct UserId(pub i64);
    pub struct User {
        id: UserId,
        name: String,
    }
}

pub mod order {
    use crate::user::UserId;
    pub struct Order {
        user_id: UserId, // Reference by ID
        items: Vec<OrderItem>,
    }
}

Dependency Injection Anti-Patterns

1. Global Service Locator

// ❌ Anti-pattern: Global state
use once_cell::sync::Lazy;

static SERVICES: Lazy<Arc<Mutex<ServiceLocator>>> = Lazy::new(|| {
    Arc::new(Mutex::new(ServiceLocator::new()))
});

// ✅ Better: Explicit dependency injection
#[derive(Clone)]
pub struct AppContext {
    pub database: Database,
    pub user_service: UserService,
}

pub async fn process_request(ctx: &AppContext, user_id: i64) -> Result<Response, Error> {
    ctx.user_service.get_user(user_id).await
}

---

Testing Anti-Patterns

Unit Testing Anti-Patterns

1. Testing Implementation Details

// ❌ Anti-pattern: Testing internals
#[test]
fn test_internal_counter() {
    let mut processor = DataProcessor::new();
    processor.increment_counter(); // Testing private state
    assert_eq!(processor.get_counter(), 1);
}

// ✅ Better: Test behavior
#[test]
fn test_data_processing_behavior() {
    let input = vec![1, 2, 3];
    let expected = vec![2, 4, 6];
    let result = process_data(&input);
    assert_eq!(result, expected);
}

2. The Liar Test

// ❌ Anti-pattern: Test that doesn't test
#[test]
fn test_user_creation() {
    let user = User::new("Alice");
    assert!(true); // Meaningless!
}

// ✅ Better: Meaningful assertions
#[test]
fn test_user_creation() {
    let user = User::new("Alice");
    assert_eq!(user.name(), "Alice");
    assert!(user.id() > 0);
}

Database Testing Anti-Patterns

1. Shared Test Database State

// ❌ Anti-pattern: Tests depend on each other
#[tokio::test]
async fn test_user_creation() {
    let pool = setup_test_db().await;
    create_user(&pool, "Alice").await.unwrap();
    // Test state leaks to other tests
}

// ✅ Better: Isolated test databases
#[tokio::test]
async fn test_user_creation() {
    let pool = setup_isolated_test_db().await;
    
    let user = create_user(&pool, "Alice").await.unwrap();
    assert_eq!(user.name, "Alice");
    
    cleanup_test_db(&pool).await;
}

---

Security Anti-Patterns

Input Validation Anti-Patterns

1. Client-Side Only Validation

// ❌ Anti-pattern: Trusting client input
async fn create_user(input: UserInput) -> Result<User, Error> {
    // Assumes client validated the input
    database::create_user(input).await
}

// ✅ Better: Server-side validation
async fn create_user(input: UserInput) -> Result<User, Error> {
    validate_email(&input.email)?;
    validate_password(&input.password)?;
    database::create_user(input).await
}

2. Blacklist Validation

// ❌ Anti-pattern: Blocking known bad patterns
fn validate_input(input: &str) -> bool {
    !input.contains("DROP") && !input.contains("DELETE")
}

// ✅ Better: Whitelist validation
fn validate_input(input: &str) -> bool {
    input.chars().all(|c| c.is_alphanumeric() || c == '_')
}

---

Performance Anti-Patterns

Memory Management Anti-Patterns

1. Unnecessary Allocations

// ❌ Anti-pattern: Creating temporary strings
fn process_strings(input: &[String]) -> Vec<String> {
    let mut result = Vec::new();
    for s in input {
        let processed = format!("processed_{}", s); // Allocation
        result.push(processed);
    }
    result
}

// ✅ Better: Minimize allocations
fn process_strings_efficient(input: &[String]) -> Vec<String> {
    input.iter()
        .map(|s| format!("processed_{}", s))
        .collect()
}

2. Clone-Heavy Processing

// ❌ Anti-pattern: Excessive cloning
fn analyze_data(data: Vec<DataPoint>) -> AnalysisResult {
    let filtered = data.clone().into_iter()
        .filter(|p| p.value > 0.0)
        .collect::<Vec<_>>();
    
    let transformed = filtered.clone().into_iter()
        .map(|p| p.value * 2.0)
        .collect::<Vec<_>>();
    
    AnalysisResult::from(transformed)
}

// ✅ Better: Chain operations
fn analyze_data_efficient(data: &[DataPoint]) -> AnalysisResult {
    let result: Vec<f64> = data.iter()
        .filter(|p| p.value > 0.0)
        .map(|p| p.value * 2.0)
        .collect();
    
    AnalysisResult::from(result)
}

---

Project-Specific Guidelines

Leptos-Specific Anti-Patterns

1. Signal Spam

// ❌ Anti-pattern: Too many signals
#[component]
fn BadComponent() -> impl IntoView {
    let signal1 = create_signal(0);
    let signal2 = create_signal("");
    let signal3 = create_signal(false);
    // ... 20 more signals
}

// ✅ Better: Group related state
#[component]
fn GoodComponent() -> impl IntoView {
    let state = create_signal(ComponentState::default());
}

2. Effect Overload

// ❌ Anti-pattern: Too many effects
create_effect(move |_| { /* ... */ });
create_effect(move |_| { /* ... */ });
create_effect(move |_| { /* ... */ });

// ✅ Better: Combine related effects
create_effect(move |_| {
    // Handle all related updates together
});

Testing Strategy

• UI Testing: Focus on integration/E2E tests, not unit tests for Leptos views
• Business Logic: Ensure all non-UI logic has unit tests
• Database Tests: Use isolated test databases for each test
• Regression Defense: Always run cargo leptos test before merging

Pre-Merge Checklist

✅ Run cargo clippy and fix all warnings
✅ Run cargo leptos test for regression testing
✅ Check formatting with cargo fmt
✅ Verify business logic unit tests (UI excluded)
✅ Validate error handling paths
✅ Check for merge conflict risks
✅ Review pattern spotting and red flags
✅ Ensure SQL SELECT changes match struct updates
✅ No hard-coded dates or AccountCtx in resources

---

Detection & Prevention Strategies

Static Analysis Configuration

# Cargo.toml
[lints.clippy]
# Encourage functional patterns
map_unwrap_or = "warn"
manual_map = "warn"
manual_filter_map = "warn"
unnecessary_fold = "warn"

# Discourage imperative patterns
needless_collect = "warn"
explicit_into_iter_loop = "warn"
manual_memcpy = "warn"

# Enforce immutability
mut_mut = "warn"
ptr_arg = "warn"

Automated Detection Tools

1. Clippy: Rust linter for common mistakes
2. SQLx: Compile-time checked SQL queries
3. cargo-audit: Security vulnerability scanning
4. cargo-outdated: Dependency version checking
5. cargo-deny: License and security compliance

Code Review Best Practices

1. Always start with praise - Acknowledge good work
2. Be specific - Point to exact lines and suggest fixes
3. Consider context - Understand why code was written this way
4. Provide examples - Show the better alternative
5. Be constructive - Focus on code, not person
6. Follow up - Ensure critical issues are addressed

---

Advanced Rust Anti-Patterns

Trait Design Anti-Patterns

1. Trait Object Overuse

// ❌ Anti-pattern: Box<dyn Trait> everywhere
struct BadDesign {
    processors: Vec<Box<dyn Processor>>,
}

impl BadDesign {
    fn process(&self) {
        for p in &self.processors {
            p.process(); // Dynamic dispatch overhead
        }
    }
}

// ✅ Better: Use generics when possible
struct BetterDesign<P: Processor> {
    processors: Vec<P>,
}

impl<P: Processor> BetterDesign<P> {
    fn process(&self) {
        for p in &self.processors {
            p.process(); // Static dispatch
        }
    }
}

2. Overly Generic Traits

// ❌ Anti-pattern: Too many generic parameters
trait OverlyGeneric<T, U, V, W> 
where
    T: Clone + Debug + Send + Sync,
    U: Default + PartialEq,
    V: Iterator<Item = W>,
    W: Display,
{
    fn process(&self, t: T, u: U, v: V) -> W;
}

// ✅ Better: Focused traits
trait Process {
    type Input;
    type Output;
    
    fn process(&self, input: Self::Input) -> Self::Output;
}

3. Missing Associated Types

// ❌ Anti-pattern: Generic parameters that should be associated
trait Container<T> {
    fn get(&self) -> &T;
}

// ✅ Better: Associated types for fixed relationships
trait Container {
    type Item;
    
    fn get(&self) -> &Self::Item;
}

Macro Anti-Patterns

1. Macro Overuse

// ❌ Anti-pattern: Macro for simple functions
macro_rules! add_one {
    ($x:expr) => {
        $x + 1
    };
}

// ✅ Better: Use functions
fn add_one(x: i32) -> i32 {
    x + 1
}

2. Unhygienic Macros

// ❌ Anti-pattern: Macro that captures variables
macro_rules! bad_macro {
    () => {
        let x = 5; // This pollutes the calling scope
    };
}

// ✅ Better: Use unique identifiers or proc macros
macro_rules! better_macro {
    ($name:ident) => {
        let $name = 5;
    };
}

Lifetime Anti-Patterns

1. Unnecessary Lifetime Annotations

// ❌ Anti-pattern: Lifetimes that can be elided
fn process<'a>(x: &'a str) -> &'a str {
    x
}

// ✅ Better: Let Rust infer
fn process(x: &str) -> &str {
    x
}

2. Self-Referential Structs

// ❌ Anti-pattern: Trying to store references to self
struct SelfReferential {
    data: String,
    ptr: Option<&'static str>, // Can't reference self.data
}

// ✅ Better: Use indices or Rc/Arc
use std::rc::Rc;

struct BetterDesign {
    data: Rc<String>,
}

---

Concurrency & Parallelism Anti-Patterns

Thread Safety Anti-Patterns

1. Shared Mutable State Without Synchronization

// ❌ Anti-pattern: Race conditions
static mut COUNTER: i32 = 0;

unsafe fn increment() {
    COUNTER += 1; // Race condition!
}

// ✅ Better: Use atomic types
use std::sync::atomic::{AtomicI32, Ordering};

static COUNTER: AtomicI32 = AtomicI32::new(0);

fn increment() {
    COUNTER.fetch_add(1, Ordering::SeqCst);
}

2. Deadlock-Prone Lock Ordering

// ❌ Anti-pattern: Inconsistent lock ordering
fn transfer_a_to_b(a: &Mutex<Account>, b: &Mutex<Account>) {
    let mut a = a.lock().unwrap();
    let mut b = b.lock().unwrap(); // Can deadlock
}

fn transfer_b_to_a(b: &Mutex<Account>, a: &Mutex<Account>) {
    let mut b = b.lock().unwrap();
    let mut a = a.lock().unwrap(); // Different order!
}

// ✅ Better: Consistent lock ordering
fn transfer(from: &Mutex<Account>, to: &Mutex<Account>) {
    // Always lock in address order
    let (first, second) = if from as *const _ < to as *const _ {
        (from, to)
    } else {
        (to, from)
    };
    
    let _guard1 = first.lock().unwrap();
    let _guard2 = second.lock().unwrap();
}

3. Mutex Poisoning Ignorance

// ❌ Anti-pattern: Always unwrapping mutex locks
let data = mutex.lock().unwrap(); // Panics if poisoned

// ✅ Better: Handle poisoned mutexes
match mutex.lock() {
    Ok(guard) => { /* use guard */ },
    Err(poisoned) => {
        // Recover or propagate
        let guard = poisoned.into_inner();
        // Fix the data
    }
}

Channel Anti-Patterns

1. Unbounded Channel Overuse

// ❌ Anti-pattern: Always using unbounded channels
let (tx, rx) = mpsc::unbounded_channel();
// Can cause memory issues if producer is faster than consumer

// ✅ Better: Use bounded channels for backpressure
let (tx, rx) = mpsc::channel(100);

2. Channel Sender Cloning

// ❌ Anti-pattern: Cloning sender for each message
for i in 0..1000 {
    let tx = tx.clone();
    thread::spawn(move || {
        tx.send(i).unwrap();
    });
}

// ✅ Better: Clone once per thread
let tx = tx.clone();
thread::spawn(move || {
    for i in 0..1000 {
        tx.send(i).unwrap();
    }
});

---

Advanced SQL Anti-Patterns

Query Optimization Anti-Patterns

1. Correlated Subqueries in SELECT

-- ❌ Anti-pattern: Subquery executes for each row
SELECT 
    u.name,
    (SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id) as order_count
FROM users u

-- ✅ Better: Use JOIN with GROUP BY
SELECT 
    u.name,
    COUNT(o.id) as order_count
FROM users u
LEFT JOIN orders o ON u.id = o.user_id
GROUP BY u.id, u.name

2. Using HAVING Instead of WHERE

-- ❌ Anti-pattern: Filtering after grouping
SELECT user_id, COUNT(*) as cnt
FROM orders
GROUP BY user_id
HAVING user_id > 100

-- ✅ Better: Filter before grouping
SELECT user_id, COUNT(*) as cnt
FROM orders
WHERE user_id > 100
GROUP BY user_id

3. Inefficient Pagination

-- ❌ Anti-pattern: Using OFFSET for large values
SELECT * FROM users
ORDER BY created_at
LIMIT 10 OFFSET 10000  -- Scans 10010 rows!

-- ✅ Better: Use cursor-based pagination
SELECT * FROM users
WHERE created_at > '2023-01-01 00:00:00'
ORDER BY created_at
LIMIT 10

4. Multiple Column IN Clauses

-- ❌ Anti-pattern: Separate IN clauses
SELECT * FROM products
WHERE category_id IN (1, 2, 3)
  AND brand_id IN (10, 20, 30)
-- Creates cartesian product in query plan

-- ✅ Better: Use compound conditions
SELECT * FROM products
WHERE (category_id, brand_id) IN ((1, 10), (2, 20), (3, 30))

Index Anti-Patterns

1. Function-Based Index Ignorance

-- ❌ Anti-pattern: Function prevents index use
SELECT * FROM users
WHERE LOWER(email) = '[email protected]'

-- ✅ Better: Create functional index or normalize data
CREATE INDEX idx_users_email_lower ON users(LOWER(email));
-- Or store emails in lowercase

2. Leading Wildcard LIKE

-- ❌ Anti-pattern: Can't use index
SELECT * FROM products
WHERE name LIKE '%phone%'

-- ✅ Better: Use full-text search or redesign
-- For SQLite: Use FTS5
CREATE VIRTUAL TABLE products_fts USING fts5(name, description);

3. Redundant Indexes

-- ❌ Anti-pattern: Index on (a) is redundant
CREATE INDEX idx_a ON table(a);
CREATE INDEX idx_a_b ON table(a, b);

-- ✅ Better: Composite index covers single column
CREATE INDEX idx_a_b ON table(a, b);
-- This index can be used for queries on 'a' alone

---

Error Handling Patterns & Anti-Patterns

Error Type Design Anti-Patterns

1. String Error Types

// ❌ Anti-pattern: Using strings for errors
fn process() -> Result<Data, String> {
    Err("Something went wrong".to_string())
}

// ✅ Better: Use proper error types
use thiserror::Error;

#[derive(Error, Debug)]
enum ProcessError {
    #[error("Invalid input: {0}")]
    InvalidInput(String),
    
    #[error("Database error")]
    Database(#[from] sqlx::Error),
}

fn process() -> Result<Data, ProcessError> {
    Err(ProcessError::InvalidInput("missing field".into()))
}

2. Error Type Proliferation

// ❌ Anti-pattern: Too many error types
enum UserError { /* ... */ }
enum OrderError { /* ... */ }
enum PaymentError { /* ... */ }
enum ShippingError { /* ... */ }

// ✅ Better: Unified error type with variants
#[derive(Error, Debug)]
enum AppError {
    #[error("User error: {0}")]
    User(String),
    
    #[error("Order error: {0}")]
    Order(String),
    
    #[error("Database error")]
    Database(#[from] sqlx::Error),
}

3. Lossy Error Conversion

// ❌ Anti-pattern: Losing error information
impl From<IoError> for AppError {
    fn from(_: IoError) -> Self {
        AppError::Generic("IO error occurred".into())
    }
}

// ✅ Better: Preserve error details
impl From<IoError> for AppError {
    fn from(err: IoError) -> Self {
        AppError::Io(err)
    }
}

Error Recovery Anti-Patterns

1. Panic on Recoverable Errors

// ❌ Anti-pattern: Panicking unnecessarily
fn get_config() -> Config {
    let file = std::fs::read_to_string("config.toml")
        .expect("Config file must exist!"); // Panic!
    
    toml::from_str(&file).expect("Invalid config!") // Panic!
}

// ✅ Better: Return Result or provide defaults
fn get_config() -> Result<Config, ConfigError> {
    let file = std::fs::read_to_string("config.toml")
        .or_else(|_| Ok(DEFAULT_CONFIG.to_string()))?;
    
    toml::from_str(&file).map_err(ConfigError::Parse)
}

2. Silent Error Suppression

// ❌ Anti-pattern: Hiding errors
fn save_user(user: &User) {
    let _ = database::insert(user); // Error ignored!
}

// ✅ Better: Handle or propagate
fn save_user(user: &User) -> Result<(), DatabaseError> {
    database::insert(user).map_err(|e| {
        error!("Failed to save user: {}", e);
        e
    })
}

---

Domain-Driven Design Anti-Patterns

Domain Modeling Anti-Patterns

1. Anemic Domain Model

// ❌ Anti-pattern: Data without behavior
struct User {
    pub id: i64,
    pub email: String,
    pub balance: f64,
}

// Service does all the work
struct UserService;
impl UserService {
    fn withdraw(&self, user: &mut User, amount: f64) {
        user.balance -= amount;
    }
}

// ✅ Better: Rich domain model
struct User {
    id: UserId,
    email: Email,
    balance: Money,
}

impl User {
    pub fn withdraw(&mut self, amount: Money) -> Result<(), DomainError> {
        if self.balance < amount {
            return Err(DomainError::InsufficientFunds);
        }
        self.balance -= amount;
        Ok(())
    }
}

2. Primitive Obsession

// ❌ Anti-pattern: Using primitives for domain concepts
fn transfer_money(from: i64, to: i64, amount: f64) { }

// ✅ Better: Domain types
#[derive(Debug, Clone, PartialEq)]
struct AccountId(i64);

#[derive(Debug, Clone, PartialEq)]
struct Money {
    amount: Decimal,
    currency: Currency,
}

fn transfer_money(from: AccountId, to: AccountId, amount: Money) { }

3. Missing Value Objects

// ❌ Anti-pattern: Inline validation
struct User {
    email: String, // Any string accepted
}

impl User {
    fn set_email(&mut self, email: String) {
        if email.contains('@') {
            self.email = email;
        }
    }
}

// ✅ Better: Value objects with invariants
#[derive(Debug, Clone)]
struct Email(String);

impl Email {
    pub fn new(value: String) -> Result<Self, ValidationError> {
        if !value.contains('@') {
            return Err(ValidationError::InvalidEmail);
        }
        Ok(Email(value))
    }
}

struct User {
    email: Email, // Always valid
}

Aggregate Design Anti-Patterns

1. Large Aggregates

// ❌ Anti-pattern: Aggregate too large
struct Order {
    items: Vec<OrderItem>,
    customer: Customer, // Full customer object
    shipping_address: Address,
    billing_address: Address,
    payment_history: Vec<Payment>,
    shipping_history: Vec<Shipment>,
    // ... 20 more fields
}

// ✅ Better: Smaller, focused aggregates
struct Order {
    id: OrderId,
    customer_id: CustomerId, // Reference only
    items: Vec<OrderItem>,
    status: OrderStatus,
}

struct Shipment {
    id: ShipmentId,
    order_id: OrderId,
    address: Address,
    status: ShipmentStatus,
}

2. Cross-Aggregate Transactions

// ❌ Anti-pattern: Modifying multiple aggregates
async fn process_order(order: &mut Order, inventory: &mut Inventory, customer: &mut Customer) {
    order.confirm();
    inventory.reserve(order.items());
    customer.add_points(order.total());
    // If any fails, inconsistent state!
}

// ✅ Better: Event-driven consistency
async fn process_order(order: &mut Order) -> Vec<DomainEvent> {
    order.confirm();
    vec![
        DomainEvent::OrderConfirmed { order_id: order.id() },
        DomainEvent::InventoryReservationRequested { /* ... */ },
        DomainEvent::CustomerPointsEarned { /* ... */ },
    ]
}

---

CI/CD Anti-Patterns

Build Pipeline Anti-Patterns

1. Missing Pre-commit Hooks

# ❌ Anti-pattern: No local validation
git commit -m "Add feature"
git push # Fails in CI

# ✅ Better: Pre-commit hooks
# .git/hooks/pre-commit
#!/bin/sh
cargo fmt --check
cargo clippy -- -D warnings
cargo test

2. Slow CI Pipelines

# ❌ Anti-pattern: Sequential steps
steps:
  - cargo build
  - cargo test
  - cargo clippy
  - cargo fmt --check

# ✅ Better: Parallel execution
jobs:
  test:
    runs-on: ubuntu-latest
    steps: [cargo test]
  
  clippy:
    runs-on: ubuntu-latest
    steps: [cargo clippy]
  
  format:
    runs-on: ubuntu-latest
    steps: [cargo fmt --check]

3. No Caching

# ❌ Anti-pattern: Rebuilding everything
steps:
  - cargo build # Downloads and compiles all deps every time

# ✅ Better: Cache dependencies
steps:
  - uses: actions/cache@v3
    with:
      path: |
        ~/.cargo/registry
        ~/.cargo/git
        target
      key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}

---

Documentation Anti-Patterns

Code Documentation Anti-Patterns

1. Missing Documentation

// ❌ Anti-pattern: No documentation
pub fn process_data(input: &[u8], flags: u32) -> Result<Vec<u8>, Error> {
    // Complex implementation
}

// ✅ Better: Comprehensive documentation
/// Processes raw data according to specified flags.
///
/// # Arguments
/// * `input` - Raw byte data to process
/// * `flags` - Processing flags (see [`ProcessFlags`])
///
/// # Returns
/// Processed data or an error if processing fails
///
/// # Examples
/// ```
/// let data = b"hello";
/// let result = process_data(data, ProcessFlags::COMPRESS)?;
/// ```
pub fn process_data(input: &[u8], flags: ProcessFlags) -> Result<Vec<u8>, Error> {
    // Implementation
}

2. Outdated Documentation

// ❌ Anti-pattern: Documentation doesn't match code
/// Returns the user's age
pub fn get_user_info(id: i64) -> UserInfo { // Returns full info!
    // ...
}

// ✅ Better: Keep docs in sync
/// Returns complete user information for the given ID
pub fn get_user_info(id: i64) -> UserInfo {
    // ...
}

3. Useless Comments

// ❌ Anti-pattern: Obvious comments
let x = 5; // Set x to 5
x += 1; // Increment x
return x; // Return x

// ✅ Better: Explain why, not what
// Use 5 as the default timeout in seconds per business requirements
let timeout = 5;

// Add processing overhead (typically 1 second)
timeout += PROCESSING_OVERHEAD;

return timeout;

---

Monitoring & Observability Anti-Patterns

Logging Anti-Patterns

1. Insufficient Logging

// ❌ Anti-pattern: No logging
fn process_payment(payment: Payment) -> Result<(), Error> {
    validate_payment(&payment)?;
    charge_card(&payment)?;
    update_database(&payment)?;
    Ok(())
}

// ✅ Better: Strategic logging
fn process_payment(payment: Payment) -> Result<(), Error> {
    info!("Processing payment: {}", payment.id);
    
    validate_payment(&payment)
        .map_err(|e| {
            error!("Payment validation failed for {}: {}", payment.id, e);
            e
        })?;
    
    debug!("Charging card for payment {}", payment.id);
    charge_card(&payment)?;
    
    update_database(&payment)?;
    info!("Payment {} processed successfully", payment.id);
    
    Ok(())
}

2. Sensitive Data in Logs

// ❌ Anti-pattern: Logging sensitive information
info!("User login: email={}, password={}", email, password);

// ✅ Better: Sanitize sensitive data
info!("User login attempt: email={}", email);
// Never log passwords, tokens, or PII

Metrics Anti-Patterns

1. Missing Business Metrics

// ❌ Anti-pattern: Only technical metrics
fn process_order(order: Order) {
    let start = Instant::now();
    // Process...
    metrics::histogram!("order.processing.time", start.elapsed());
}

// ✅ Better: Include business metrics
fn process_order(order: Order) {
    let start = Instant::now();
    
    // Process...
    
    metrics::histogram!("order.processing.time", start.elapsed());
    metrics::increment_counter!("orders.processed");
    metrics::histogram!("order.value", order.total());
    metrics::increment_counter!("orders.by_type", "type" => order.order_type());
}

---

Resource Management Anti-Patterns

File Handle Anti-Patterns

1. Leaking File Handles

// ❌ Anti-pattern: Manual resource management
use std::fs::File;

fn read_file() -> Result<String, Error> {
    let mut file = File::open("data.txt")?;
    let mut contents = String::new();
    file.read_to_string(&mut contents)?;
    // File handle leaked if error occurs
    Ok(contents)
}

// ✅ Better: RAII handles cleanup
fn read_file() -> Result<String, Error> {
    std::fs::read_to_string("data.txt").map_err(Into::into)
}

Connection Pool Anti-Patterns

1. Pool Exhaustion

// ❌ Anti-pattern: Not returning connections
async fn bad_query(pool: &SqlitePool) {
    let mut conn = pool.acquire().await.unwrap();
    // Connection held for entire function scope
    do_something_else().await;
    query(&mut conn).await;
}

// ✅ Better: Minimize connection hold time
async fn good_query(pool: &SqlitePool) {
    do_something_else().await;
    
    let result = {
        let mut conn = pool.acquire().await?;
        query(&mut conn).await
    }; // Connection returned to pool
    
    result
}

---

Dependency Management Anti-Patterns

Cargo.toml Anti-Patterns

1. Wildcard Version Dependencies

# ❌ Anti-pattern: Accepting any version
[dependencies]
serde = "*"
tokio = ">=0.1"

# ✅ Better: Pin to major version
[dependencies]
serde = "1.0"
tokio = "1.35"

2. Git Dependencies in Production

# ❌ Anti-pattern: Unstable git dependencies
[dependencies]
my-lib = { git = "https://github.com/user/lib.git" }

# ✅ Better: Use crates.io or specific commits
[dependencies]
my-lib = "0.1.2"
# Or if you must use git:
my-lib = { git = "https://github.com/user/lib.git", rev = "abc123" }

3. Feature Flag Explosion

# ❌ Anti-pattern: Too many feature combinations
[features]
default = ["feature1", "feature2", "feature3"]
feature1 = ["dep1", "dep2"]
feature2 = ["dep3", "dep4"]
feature3 = ["dep5", "dep6"]
all = ["feature1", "feature2", "feature3", "feature4", "feature5"]
# 32 possible combinations!

# ✅ Better: Minimal, orthogonal features
[features]
default = ["std"]
std = []
async = ["tokio", "futures"]
serialize = ["serde"]

4. Transitive Dependency Ignorance

# ❌ Anti-pattern: Not checking transitive deps
[dependencies]
big-framework = "1.0" # Pulls in 200 dependencies!

# ✅ Better: Audit dependencies
# Run: cargo tree
# Run: cargo audit
# Consider alternatives with fewer dependencies

Workspace Anti-Patterns

1. No Workspace for Multi-Crate Projects

# ❌ Anti-pattern: Separate Cargo.toml with duplicated deps
# lib1/Cargo.toml
[dependencies]
serde = "1.0.150"

# lib2/Cargo.toml
[dependencies]
serde = "1.0.160" # Version mismatch!

# ✅ Better: Use workspace
# Cargo.toml (root)
[workspace]
members = ["lib1", "lib2"]

[workspace.dependencies]
serde = "1.0.160"

# lib1/Cargo.toml
[dependencies]
serde = { workspace = true }

2. Circular Dependencies Between Crates

# ❌ Anti-pattern: Crates depend on each other
# crate-a/Cargo.toml
[dependencies]
crate-b = { path = "../crate-b" }

# crate-b/Cargo.toml
[dependencies]
crate-a = { path = "../crate-a" } # Circular!

# ✅ Better: Extract common functionality
# crate-common/Cargo.toml
# (no dependencies on siblings)

# crate-a/Cargo.toml
[dependencies]
crate-common = { path = "../crate-common" }

# crate-b/Cargo.toml
[dependencies]
crate-common = { path = "../crate-common" }

Version Management Anti-Patterns

1. Never Updating Dependencies

# ❌ Anti-pattern: Using 2-year-old versions
# Cargo.lock not updated in years

# ✅ Better: Regular updates with testing
cargo update --dry-run  # See what would update
cargo outdated         # Check for newer versions
cargo upgrade --workspace --dry-run  # Preview updates

2. Updating Everything at Once

# ❌ Anti-pattern: Massive update PR
cargo update  # Updates 50 dependencies at once

# ✅ Better: Incremental updates
# Update one category at a time
cargo update -p serde
# Test
cargo update -p tokio
# Test

---

Code Generation Anti-Patterns

Build Script Anti-Patterns

1. Complex Logic in build.rs

// ❌ Anti-pattern: Business logic in build script
// build.rs
fn main() {
    // 500 lines of complex generation logic
    generate_api_client();
    compile_shaders();
    process_assets();
    validate_configs();
}

// ✅ Better: Simple build script, logic in separate crate
// build.rs
fn main() {
    codegen::generate_all();
}

// codegen/src/lib.rs - Testable, reusable
pub fn generate_all() {
    generate_api_client();
    // etc.
}

2. Generated Code in Version Control

// ❌ Anti-pattern: Committing generated files
// src/generated.rs (in git)
// THIS FILE IS AUTO-GENERATED
pub struct GeneratedApi { /* ... */ }

// ✅ Better: Generate at build time
// build.rs
fn main() {
    let out_dir = env::var("OUT_DIR").unwrap();
    let dest_path = Path::new(&out_dir).join("generated.rs");
    // Generate code
}

// src/lib.rs
include!(concat!(env!("OUT_DIR"), "/generated.rs"));

3. Non-Deterministic Generation

// ❌ Anti-pattern: Different output each time
fn generate_code() -> String {
    format!("// Generated at {:?}", SystemTime::now()) // Changes every build!
}

// ✅ Better: Deterministic output
fn generate_code() -> String {
    format!("// Generated from schema version {}", SCHEMA_VERSION)
}

Macro Code Generation Anti-Patterns

1. Unmaintainable Proc Macros

// ❌ Anti-pattern: Complex proc macro without structure
#[proc_macro_derive(MyDerive)]
pub fn derive(input: TokenStream) -> TokenStream {
    // 1000 lines of string manipulation
    let output = format!("impl MyTrait for {} {{ ... }}", name);
    output.parse().unwrap()
}

// ✅ Better: Use syn and quote, structured approach
#[proc_macro_derive(MyDerive)]
pub fn derive(input: TokenStream) -> TokenStream {
    let input = parse_macro_input!(input as DeriveInput);
    let name = &input.ident;
    
    quote! {
        impl MyTrait for #name {
            // Implementation
        }
    }.into()
}

2. Error Handling in Macros

// ❌ Anti-pattern: Panic in macro
#[proc_macro]
pub fn my_macro(input: TokenStream) -> TokenStream {
    let data = parse_data(input).expect("Invalid input!"); // Panic!
    generate(data)
}

// ✅ Better: Compile-time errors
#[proc_macro]
pub fn my_macro(input: TokenStream) -> TokenStream {
    let data = match parse_data(input) {
        Ok(data) => data,
        Err(err) => return err.to_compile_error().into(),
    };
    generate(data)
}

SQLx Compile-Time Query Anti-Patterns

1. Offline Mode Misuse

// ❌ Anti-pattern: Not saving query metadata
// Requires database at compile time
sqlx::query!("SELECT * FROM users WHERE id = ?", id)
    .fetch_one(pool)
    .await?;

// ✅ Better: Use offline mode for CI/CD
// First: cargo sqlx prepare
// This creates .sqlx directory with query metadata
// Then queries work without database at compile time

2. Dynamic Query Strings with query!

// ❌ Anti-pattern: Trying to use dynamic SQL with query!
let table = "users";
let query = format!("SELECT * FROM {}", table);
sqlx::query!(query) // Won't compile!
    .fetch_all(pool)
    .await?;

// ✅ Better: Use query_as for dynamic queries
let query = format!("SELECT * FROM {}", table);
sqlx::query_as::<_, User>(&query)
    .fetch_all(pool)
    .await?;

---

Team Collaboration Anti-Patterns

Code Review Anti-Patterns

1. Rubber Stamp Reviews

# ❌ Anti-pattern: Meaningless approvals
"LGTM" 
"Approved"
"+1"

# ✅ Better: Substantive feedback
"Great implementation of the caching layer! I especially like how you 
handled cache invalidation in lines 45-52. 

One suggestion: Consider adding a metric for cache hit rate on line 67.

Question: Why did you choose 5 minutes for the TTL? Is this based on 
our traffic patterns?"

2. Nitpick-Only Reviews

# ❌ Anti-pattern: Only style comments
"Use `&str` instead of `&String` on line 12"
"Missing comma on line 45"
"Variable should be snake_case"

# ✅ Better: Balance style with substance
"Architecture: The separation between domain and infrastructure layers 
is clean. Good use of dependency injection.

Logic: The retry mechanism could cause issues under high load. Consider
exponential backoff.

Style: A few Clippy warnings to address (see inline comments)."

3. Delayed Reviews

# ❌ Anti-pattern: Reviews after 3 days
PR opened: Monday
First review: Thursday
Merge: Next Monday

# ✅ Better: Timely reviews
- Review within 4 hours for small PRs (<100 lines)
- Review within 1 day for medium PRs (<500 lines)
- Schedule pairing session for large PRs

Pull Request Anti-Patterns

1. Massive PRs

# ❌ Anti-pattern: 2000+ line PR
- Changed 45 files
- Mixed features, refactoring, and bug fixes
- Impossible to review properly

# ✅ Better: Small, focused PRs
PR 1: Refactor user module (200 lines)
PR 2: Add email validation (100 lines)
PR 3: Fix login bug (50 lines)

2. Poor PR Descriptions

# ❌ Anti-pattern: Vague description
Title: "Fix stuff"
Description: "Fixed the bug we discussed"

# ✅ Better: Detailed description
Title: "Fix race condition in payment processing"

## Problem
Payment processing could fail silently when two requests arrived simultaneously.

## Solution
Added mutex protection around the critical section in process_payment().

## Testing
- Added unit test for concurrent payments
- Tested with 100 concurrent requests in staging

## Breaking Changes
None

Fixes #1234

3. No PR Template

# ✅ Good: PR Template (.github/pull_request_template.md)
## Description
Brief description of changes

## Type of Change
- [ ] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update

## Testing
- [ ] Unit tests pass
- [ ] Integration tests pass
- [ ] Manual testing completed

## Checklist
- [ ] Code follows style guidelines
- [ ] Self-review completed
- [ ] Documentation updated
- [ ] No new warnings

Communication Anti-Patterns

1. Acronym Soup

// ❌ Anti-pattern: Unexplained acronyms
// TODO: Update the DRF to handle ACL for the PSK
// after the BGP is configured with the new MTU

// ✅ Better: Clear communication
// TODO: Update the Django Rest Framework to handle Access Control Lists
// for the Pre-Shared Key after the Border Gateway Protocol is configured
// with the new Maximum Transmission Unit

2. Knowledge Silos

// ❌ Anti-pattern: One person knows everything
// "Only Alice knows how the payment system works"
// "Bob is the only one who understands the deployment"

// ✅ Better: Shared knowledge
// - Documentation in code
// - Pair programming
// - Rotation of responsibilities
// - Knowledge sharing sessions

---

Git Merge-Friendly Code Patterns

File Organization for Minimal Conflicts

1. One Component Per File

// ❌ Anti-pattern: Multiple components in one file
// src/components.rs - Everyone edits this file!
pub struct UserComponent { }
pub struct OrderComponent { }
pub struct PaymentComponent { }

// ✅ Better: Separate files
// src/components/user.rs
pub struct UserComponent { }

// src/components/order.rs
pub struct OrderComponent { }

// src/components/payment.rs
pub struct PaymentComponent { }

// src/components/mod.rs
pub mod user;
pub mod order;
pub mod payment;

2. Vertical Code Organization

// ❌ Anti-pattern: Horizontal organization (merge conflicts!)
use crate::{user::User, order::Order, payment::Payment, shipping::Shipping, inventory::Inventory};

const USER_TIMEOUT: u64 = 30, ORDER_TIMEOUT: u64 = 60, PAYMENT_TIMEOUT: u64 = 90;

// ✅ Better: Vertical organization
use crate::user::User;
use crate::order::Order;
use crate::payment::Payment;
use crate::shipping::Shipping;
use crate::inventory::Inventory;

const USER_TIMEOUT: u64 = 30;
const ORDER_TIMEOUT: u64 = 60;
const PAYMENT_TIMEOUT: u64 = 90;

3. Append-Only Patterns

// ❌ Anti-pattern: Inserting in the middle
enum Status {
    Active,
    Inactive,  // Someone adds here
    Pending,   // Someone else adds here - CONFLICT!
    Deleted,
}

// ✅ Better: Append at the end
enum Status {
    Active,
    Inactive,
    Deleted,
    // New variants go here
    Pending,    // Added by developer 1
    Processing, // Added by developer 2 - No conflict!
}

Configuration for Merge-Friendly Code

1. Trailing Commas

// ❌ Anti-pattern: No trailing comma
let config = Config {
    host: "localhost",
    port: 8080  // Adding field here causes conflict
}

// ✅ Better: Always use trailing commas
let config = Config {
    host: "localhost",
    port: 8080,  // Trailing comma
    // New fields can be added here without conflicts
}

2. One Statement Per Line

// ❌ Anti-pattern: Multiple statements per line
use std::{collections::HashMap, sync::Arc, thread, time::Duration};

// ✅ Better: One import per line
use std::collections::HashMap;
use std::sync::Arc;
use std::thread;
use std::time::Duration;

3. Consistent Formatting

# .rustfmt.toml - Enforce consistent formatting
edition = "2021"
max_width = 100
hard_tabs = false
tab_spaces = 4
newline_style = "Unix"
use_small_heuristics = "Default"
imports_granularity = "Crate"
group_imports = "StdExternalCrate"

Structural Patterns for Conflict Avoidance

1. Builder Pattern for Complex Structs

// ❌ Anti-pattern: Constructor with many parameters
impl User {
    pub fn new(name: String, email: String, age: u32, role: Role, status: Status) -> Self {
        // Adding parameters causes conflicts
    }
}

// ✅ Better: Builder pattern
impl User {
    pub fn builder() -> UserBuilder {
        UserBuilder::default()
    }
}

impl UserBuilder {
    pub fn name(mut self, name: String) -> Self {
        self.name = Some(name);
        self
    }
    
    pub fn email(mut self, email: String) -> Self {
        self.email = Some(email);
        self
    }
    // New methods can be added without conflicts
}

2. Extension Traits

// ❌ Anti-pattern: Modifying core structs
impl User {
    fn new_method(&self) { } // Everyone adds methods here
}

// ✅ Better: Extension traits in separate modules
// src/extensions/user_auth.rs
pub trait UserAuthExt {
    fn authenticate(&self) -> bool;
}

impl UserAuthExt for User {
    fn authenticate(&self) -> bool { /* ... */ }
}

// src/extensions/user_profile.rs
pub trait UserProfileExt {
    fn update_profile(&mut self, profile: Profile);
}

3. Event-Driven Architecture

// ❌ Anti-pattern: Direct coupling
fn process_order(order: Order) {
    update_inventory(order.items);
    charge_payment(order.payment);
    send_notification(order.user);
    update_analytics(order);
    // Everyone adds calls here - conflicts!
}

// ✅ Better: Event-driven
fn process_order(order: Order) -> Vec<Event> {
    vec![
        Event::OrderProcessed { order_id: order.id },
    ]
}

// Handlers in separate files
// src/handlers/inventory.rs
pub fn handle_order_processed(event: Event) { }

// src/handlers/payment.rs
pub fn handle_order_processed(event: Event) { }

---

Leptos-Specific Anti-Patterns

Signal Management Anti-Patterns

1. Signal Proliferation

// ❌ Anti-pattern: Too many signals
#[component]
fn BadComponent() -> impl IntoView {
    let (name, set_name) = create_signal(String::new());
    let (email, set_email) = create_signal(String::new());
    let (age, set_age) = create_signal(0);
    let (address, set_address) = create_signal(String::new());
    let (phone, set_phone) = create_signal(String::new());
    // 10 more signals...
    
    view! { /* ... */ }
}

// ✅ Better: Group related state
#[derive(Clone, Debug, Default)]
struct UserForm {
    name: String,
    email: String,
    age: u32,
    address: String,
    phone: String,
}

#[component]
fn GoodComponent() -> impl IntoView {
    let (form, set_form) = create_signal(UserForm::default());
    
    view! { /* ... */ }
}

2. Effect Overuse

// ❌ Anti-pattern: Too many effects
#[component]
fn BadEffects() -> impl IntoView {
    let (count, set_count) = create_signal(0);
    
    create_effect(move |_| {
        log!("Count changed to: {}", count.get());
    });
    
    create_effect(move |_| {
        if count.get() > 10 {
            log!("Count is high!");
        }
    });
    
    create_effect(move |_| {
        document().set_title(&format!("Count: {}", count.get()));
    });
    
    // ✅ Better: Combine related effects or use derived signals
    let display_count = move || format!("Count: {}", count.get());
    let is_high = move || count.get() > 10;
    
    create_effect(move |_| {
        log!("Count: {}, High: {}", count.get(), is_high());
        document().set_title(&display_count());
    });
    
    view! { /* ... */ }
}

3. Memory Leaks with Signals

// ❌ Anti-pattern: Not cleaning up
#[component]
fn LeakyComponent() -> impl IntoView {
    let (data, set_data) = create_signal(Vec::<String>::new());
    
    // Spawning without cleanup
    spawn_local(async move {
        loop {
            let new_data = fetch_data().await;
            set_data.update(|d| d.extend(new_data));
            sleep(Duration::from_secs(1)).await;
        }
    });
    
    view! { /* ... */ }
}

// ✅ Better: Use on_cleanup
#[component]
fn CleanComponent() -> impl IntoView {
    let (data, set_data) = create_signal(Vec::<String>::new());
    let (stop_signal, set_stop) = create_signal(false);
    
    let handle = spawn_local(async move {
        while !stop_signal.get() {
            let new_data = fetch_data().await;
            set_data.update(|d| d.extend(new_data));
            sleep(Duration::from_secs(1)).await;
        }
    });
    
    on_cleanup(move || {
        set_stop.set(true);
    });
    
    view! { /* ... */ }
}

Component Design Anti-Patterns

1. Prop Drilling

// ❌ Anti-pattern: Passing props through multiple levels
#[component]
fn App() -> impl IntoView {
    let user = create_signal(User::default());
    view! {
        <Layout user=user>
            <Dashboard user=user>
                <UserProfile user=user />
            </Dashboard>
        </Layout>
    }
}

// ✅ Better: Use context
#[component]
fn App() -> impl IntoView {
    let user = create_signal(User::default());
    provide_context(user);
    
    view! {
        <Layout>
            <Dashboard>
                <UserProfile />
            </Dashboard>
        </Layout>
    }
}

#[component]
fn UserProfile() -> impl IntoView {
    let user = use_context::<Signal<User>>().unwrap();
    view! { /* ... */ }
}

2. Component Bloat

// ❌ Anti-pattern: Monster component with 500+ lines
#[component]
fn MonsterComponent() -> impl IntoView {
    // 50 signals
    // 20 effects
    // 300 lines of view logic
    
    view! {
        // Huge nested structure
    }
}

// ✅ Better: Break into smaller components
#[component]
fn UserDashboard() -> impl IntoView {
    view! {
        <Header />
        <UserStats />
        <RecentActivity />
        <Footer />
    }
}

Server Function Anti-Patterns

1. Blocking Operations in Server Functions

// ❌ Anti-pattern: Blocking I/O in async context
#[server(GetUser, "/api")]
pub async fn get_user(id: i64) -> Result<User, ServerFnError> {
    let file = std::fs::read_to_string("users.json")?; // Blocking!
    // Parse and return user
    Ok(user)
}

// ✅ Better: Use async I/O
#[server(GetUser, "/api")]
pub async fn get_user(id: i64) -> Result<User, ServerFnError> {
    let file = tokio::fs::read_to_string("users.json").await?;
    // Parse and return user
    Ok(user)
}

2. Large Payload Server Functions

// ❌ Anti-pattern: Returning huge amounts of data
#[server(GetAllUsers, "/api")]
pub async fn get_all_users() -> Result<Vec<User>, ServerFnError> {
    // Returns 10,000 users with full details
    sqlx::query_as::<_, User>("SELECT * FROM users")
        .fetch_all(&pool)
        .await
}

// ✅ Better: Paginate and filter
#[server(GetUsers, "/api")]
pub async fn get_users(
    page: usize,
    limit: usize,
) -> Result<PagedResponse<User>, ServerFnError> {
    let offset = page * limit;
    let users = sqlx::query_as::<_, User>(
        "SELECT id, name, email FROM users LIMIT ? OFFSET ?"
    )
    .bind(limit as i64)
    .bind(offset as i64)
    .fetch_all(&pool)
    .await?;
    
    Ok(PagedResponse { data: users, page, total })
}

---

SQLx-Specific Anti-Patterns

Query Construction Anti-Patterns

1. String Concatenation in Queries

// ❌ Anti-pattern: SQL injection vulnerability
let table = user_input;
let query = format!("SELECT * FROM {}", table);
sqlx::query(&query).fetch_all(&pool).await?;

// ✅ Better: Use safe alternatives
match user_input {
    "users" => sqlx::query("SELECT * FROM users").fetch_all(&pool).await,
    "orders" => sqlx::query("SELECT * FROM orders").fetch_all(&pool).await,
    _ => Err(Error::InvalidTable),
}

2. Mismatched Bindings

// ❌ Anti-pattern: Wrong number of bindings
let user = sqlx::query_as::<_, User>(
    "SELECT * FROM users WHERE id = ? AND email = ?"
)
.bind(id) // Missing second binding!
.fetch_one(&pool)
.await?;

// ✅ Better: Match bindings to placeholders
let user = sqlx::query_as::<_, User>(
    "SELECT * FROM users WHERE id = ? AND email = ?"
)
.bind(id)
.bind(email)
.fetch_one(&pool)
.await?;

3. Using query! with Dynamic SQL

// ❌ Anti-pattern: query! needs static SQL
let column = "name";
sqlx::query!(
    &format!("SELECT {} FROM users", column) // Won't compile!
)
.fetch_all(&pool)
.await?;

// ✅ Better: Use query_as for dynamic SQL
let query = format!("SELECT {} FROM users", column);
sqlx::query_as::<_, UserName>(&query)
    .fetch_all(&pool)
    .await?;

Transaction Anti-Patterns

1. Long-Running Transactions

// ❌ Anti-pattern: Transaction held too long
let mut tx = pool.begin().await?;

let users = sqlx::query_as::<_, User>("SELECT * FROM users")
    .fetch_all(&mut tx)
    .await?;

// Processing that takes 30 seconds
for user in users {
    expensive_operation(user).await;
}

sqlx::query("UPDATE stats SET processed = true")
    .execute(&mut tx)
    .await?;

tx.commit().await?;

// ✅ Better: Minimize transaction scope
let users = sqlx::query_as::<_, User>("SELECT * FROM users")
    .fetch_all(&pool)
    .await?;

// Process outside transaction
for user in users {
    expensive_operation(user).await;
}

// Quick transaction for update
let mut tx = pool.begin().await?;
sqlx::query("UPDATE stats SET processed = true")
    .execute(&mut tx)
    .await?;
tx.commit().await?;

2. Missing Rollback Handling

// ❌ Anti-pattern: No explicit rollback
let mut tx = pool.begin().await?;

if let Err(e) = process(&mut tx).await {
    // Transaction left in limbo
    return Err(e);
}

tx.commit().await?;

// ✅ Better: Explicit rollback on error
let mut tx = pool.begin().await?;

match process(&mut tx).await {
    Ok(result) => {
        tx.commit().await?;
        Ok(result)
    }
    Err(e) => {
        tx.rollback().await?;
        Err(e)
    }
}

Migration Anti-Patterns

1. Destructive Migrations

-- ❌ Anti-pattern: Data loss migration
-- V1__drop_column.sql
ALTER TABLE users DROP COLUMN important_data;

-- ✅ Better: Safe migrations
-- V1__deprecate_column.sql
ALTER TABLE users ADD COLUMN important_data_backup TEXT;
UPDATE users SET important_data_backup = important_data;
-- Later migration after verifying backup
-- V2__remove_deprecated.sql
ALTER TABLE users DROP COLUMN important_data;

2. Non-Idempotent Migrations

-- ❌ Anti-pattern: Fails if run twice
INSERT INTO settings (key, value) VALUES ('version', '1.0');

-- ✅ Better: Idempotent operations
INSERT INTO settings (key, value) 
VALUES ('version', '1.0')
ON CONFLICT (key) DO NOTHING;

Pool Configuration Anti-Patterns

1. Incorrect Pool Sizing

// ❌ Anti-pattern: Pool too small or too large
let pool = SqlitePoolOptions::new()
    .max_connections(1) // Too small, bottleneck!
    // or
    .max_connections(1000) // Too large for SQLite!
    .connect("sqlite:database.db")
    .await?;

// ✅ Better: Appropriate pool size
let pool = SqlitePoolOptions::new()
    .max_connections(5) // SQLite works best with fewer connections
    .connect("sqlite:database.db")
    .await?;

2. No Connection Timeout

// ❌ Anti-pattern: Infinite timeout
let pool = SqlitePoolOptions::new()
    .connect("sqlite:database.db")
    .await?;

// ✅ Better: Set reasonable timeouts
let pool = SqlitePoolOptions::new()
    .max_connections(5)
    .connect_timeout(Duration::from_secs(5))
    .acquire_timeout(Duration::from_secs(5))
    .idle_timeout(Duration::from_secs(10))
    .max_lifetime(Duration::from_secs(1800))
    .connect("sqlite:database.db")
    .await?;

---

Anti-Pattern Detection Strategy & Reviewer Mindset

The Core Review Philosophy

Code reviews should focus on improving overall system health rather than achieving perfection. According to Google's engineering practices, reviewers should approve changes that definitively improve the codebase, even when imperfect. "There is no such thing as perfect code—there is only better code."

Systematic Detection Approach

Layer 1: Pre-Commit Automated Detection

# .pre-commit-config.yaml
- repo: local
  hooks:
    - id: clippy-check
      name: Clippy
      entry: cargo clippy -- --deny clippy::correctness --warn clippy::perf
      language: system
      files: '\.rs
    
    - id: sqlcheck
      name: SQL Anti-Pattern Check
      entry: sqlcheck --risk-level 3
      language: system
      files: '\.sql

Layer 2: Pattern Recognition Algorithm

1. Scan for Signatures: Look for method call patterns, suspicious control flow
2. Context Analysis: Identify misplaced or misused patterns
3. Semantic Evaluation: Verify the pattern achieves its intended purpose
4. Impact Assessment: Evaluate consequences if left unchanged
5. Solution Generation: Provide clear, actionable refactors

Layer 3: Human Review Focus Areas

• Design Coherence: Does the change fit the overall architecture?
• Business Logic: Is the implementation correct for the domain?
• System Integration: How does this affect other components?
• Future Maintenance: Will this be easy to modify?

Review Time Management

Google's One Business Day Rule: Reviewers must respond within one business day maximum, with optimal flow achieved through multiple review rounds within a single day. Use "LGTM with comments" for minor issues when trusting the developer to handle remaining concerns.

What to Look for in Every Review

1. Every Line Examination

• Read every line of code you're assigned to review
• Understand not just what it does but how it fits the broader system
• Check for both obvious and subtle issues

2. Technical Facts Over Preferences

• Style guides are absolute authorities
• Personal preferences yield to team conventions
• Focus on objective improvements

3. Constructive Feedback Pattern

# Good Review Comment
"This mutex usage could lead to deadlock when combined with the 
transaction in line 45. Consider using `try_lock()` with timeout, 
or restructure to acquire locks in consistent order. See our 
deadlock prevention guide: [link]"

# Poor Review Comment
"This code is confusing."

---

Rust-Specific Detection Patterns

Clippy's 750+ Lints Hierarchy

Critical (Deny by Default) - clippy::correctness

These represent actual bugs that must be fixed:

// ❌ Float comparison without epsilon
if x == 0.1 { }  // clippy::float_cmp

// ✅ Correct approach
if (x - 0.1).abs() < f64::EPSILON { }

Performance (Warn) - clippy::perf

Search for these patterns:

• .clone() calls - verify each is necessary
• Manual indexing instead of iterators
• String building with repeated allocations
• Unnecessary collect() breaking lazy evaluation

Complexity (Warn) - clippy::complexity

Identifies unnecessarily complex code:

// ❌ Nested if statements
if x {
    if y {
        do_something();
    }
}

// ✅ Simplified
if x && y {
    do_something();
}

Rust API Guidelines Checklist

Naming Conventions (RFC 430)

• Types/Traits: UpperCamelCase
• Functions/Variables: snake_case
• Constants: SCREAMING_SNAKE_CASE
• Conversion Methods:
  • as_*: Free conversions
  • to_*: Expensive conversions
  • into_*: Variable-cost conversions

Trait Implementation Requirements

Every data structure should be evaluated for:

• Copy, Clone, Debug, Display, Default
• Collections: FromIterator, Extend
• Map keys: Hash, Eq, PartialEq
• Error types: std::error::Error + Send + Sync + 'static

Documentation Mandates

• Every public API item needs examples
• Fallible examples use ? not unwrap
• Unsafe code requires safety documentation
• Module-level docs provide usage overview

---

SQLite3-Specific Optimization Patterns

Critical Performance Settings

1. Enable WAL Mode Immediately

PRAGMA journal_mode = WAL;
-- Doubles write speed, allows concurrent readers/writers

2. Query Planner Rules

SQLite can only use an index if:

• Initial columns appear in WHERE with equality operators
• Rightmost column can use inequalities
• No gaps in column usage

-- Index: CREATE INDEX idx ON t(a,b,c,d);

-- ✅ Can use full index
WHERE a=5 AND b=10 AND c=20 AND d>100

-- ❌ Can only use 'a' and 'b' (gap at 'c')
WHERE a=5 AND b=10 AND d>100

3. WITHOUT ROWID Optimization

Use when:

• PRIMARY KEY is non-integer or composite
• Rows < 200 bytes (1/20th of 4KB page)
• Can save 50% space, double speed

-- ✅ Good candidate
CREATE TABLE events (
    timestamp INTEGER,
    sensor_id INTEGER,
    value REAL,
    PRIMARY KEY (timestamp, sensor_id)
) WITHOUT ROWID;

-- ❌ Bad candidate (single INTEGER PRIMARY KEY)
CREATE TABLE users (
    id INTEGER PRIMARY KEY,
    -- Better as normal rowid table
);

4. Essential PRAGMA Settings

-- Run before closing connection
PRAGMA optimize;

-- Increase cache (2MB)
PRAGMA cache_size = -2048;

-- Memory temp storage
PRAGMA temp_store = MEMORY;

-- Enable foreign keys (disabled by default!)
PRAGMA foreign_keys = ON;

---

SQL Anti-Pattern Detection Strategies

The N+1 Query Pattern (Most Critical)

Detection Signs:

• Loops containing single-record queries
• Multiple similar queries differing only in parameters
• ORM lazy loading in iterations

// ❌ N+1 Pattern
for user in users {
    let orders = sqlx::query_as::<_, Order>(
        "SELECT * FROM orders WHERE user_id = ?"
    ).bind(user.id).fetch_all(pool).await?;
}

// ✅ Single Query Solution
let all_data = sqlx::query_as::<_, UserWithOrders>(
    "SELECT u.*, o.* FROM users u 
     LEFT JOIN orders o ON u.id = o.user_id"
).fetch_all(pool).await?;

Query Plan Analysis Method

1. Read plans right-to-left following data flow
2. Identify high-cost operations (>50% of total)
3. Compare estimated vs actual rows (>10x difference = problem)
4. Watch for red flags:
   • Table scans on large tables
   • Hash matches on large datasets
   • Memory spills in sorts
   • High logical reads

Implicit Conversion Detection

-- ❌ Type mismatch prevents index usage
WHERE varchar_column = 123  -- Integer literal

-- ✅ Matching types
WHERE varchar_column = '123'

---

Advanced Detection Patterns

Memory Leak Indicators in Rust

Look for:

• Rc cycles without Weak references
• spawn without JoinHandle management
• Channels without proper cleanup
• Global state with Arc<Mutex<>> accumulating data

Concurrency Red Flags

// ❌ Deadlock-prone pattern
let guard1 = mutex1.lock().unwrap();
let guard2 = mutex2.lock().unwrap(); // Different order elsewhere = deadlock

// ✅ Consistent lock ordering
let (first, second) = if &mutex1 as *const _ < &mutex2 as *const _ {
    (&mutex1, &mutex2)
} else {
    (&mutex2, &mutex1)
};

Async Anti-Pattern Recognition

// ❌ Blocking in async context
async fn bad() {
    std::thread::sleep(Duration::from_secs(1)); // Blocks executor!
}

// ✅ Proper async
async fn good() {
    tokio::time::sleep(Duration::from_secs(1)).await;
}

---

Review Velocity Optimization

The "LGTM with Comments" Strategy

Use when:

• Core functionality is correct
• Only minor issues remain
• Developer is trusted to address comments
• Blocking would delay critical features

Example:

LGTM with the following to address:
- Add error handling for the edge case in line 45
- Consider extracting the complex logic in process_data() to a helper
- Update documentation for the new parameter

Please address before next deployment.

Draft PR Early Feedback

Encourage developers to:

1. Open draft PRs for architectural feedback
2. Mark specific areas needing attention
3. Convert to ready when core logic is complete

Automated vs Human Division

Automated Tools Handle:

• Formatting (rustfmt)
• Style violations (clippy)
• Security vulnerabilities (cargo-audit)
• SQL anti-patterns (sqlcheck)
• Test coverage metrics

Humans Focus On:

• Architecture appropriateness
• Business logic correctness
• Performance trade-offs
• Error handling strategy
• Documentation clarity
• Future maintainability

---

Code Readability & Refactoring Guidelines

The Readability Principle

Code is read 10x more often than it's written. Google's research shows developers spend 70% of their time reading code vs 30% writing it. Therefore, optimize for readability over cleverness. As Martin Fowler states: "Any fool can write code that a computer can understand. Good programmers write code that humans can understand."

Cognitive Load Theory in Code

Intrinsic Load (Essential Complexity)

• Business logic requirements
• Algorithm complexity
• Domain constraints

Extraneous Load (Accidental Complexity) - MINIMIZE THIS

• Poor naming
• Inconsistent patterns
• Unnecessary abstraction
• Hidden dependencies
• Surprising behavior

Managing Cognitive Load

// ❌ High cognitive load
fn p(d: Vec<(i32, i32)>) -> Vec<i32> {
    d.iter().filter(|x| x.0 > 0).map(|x| x.0 * x.1).collect()
}

// ✅ Low cognitive load
fn calculate_positive_products(pairs: Vec<(i32, i32)>) -> Vec<i32> {
    pairs
        .iter()
        .filter(|(first, _)| *first > 0)
        .map(|(first, second)| first * second)
        .collect()
}

Readability Metrics & Indicators

1. The 5-Second Rule

A developer should understand what a function does within 5 seconds of reading its signature and first few lines.

2. The WTF/Minute Metric

Count "What The F***" moments during code review. Lower is better.

3. The Squint Test

If you have to squint or lean forward to understand code, it needs refactoring.

4. Line Length & Complexity

• Cyclomatic Complexity: Should be ≤10 per function
• Line Length: 80-100 characters max
• Function Length: 40 lines ideal, 80 lines max
• Nesting Depth: Maximum 3 levels

---

Refactoring Triggers & Patterns

When to Refactor (The Rule of Three)

1. First time: Just write the code
2. Second time: Duplicate with reluctance
3. Third time: Refactor to remove duplication

Code Smells That Demand Refactoring

1. Long Functions

// ❌ Code Smell: 100+ line function doing everything
fn process_order(order: Order) -> Result<Receipt, Error> {
    // Validation logic (20 lines)
    // Inventory check (30 lines)
    // Payment processing (40 lines)
    // Email sending (20 lines)
    // Database updates (30 lines)
}

// ✅ Refactored: Composed of focused functions
fn process_order(order: Order) -> Result<Receipt, Error> {
    validate_order(&order)?;
    check_inventory(&order)?;
    let payment = process_payment(&order)?;
    send_confirmation_email(&order, &payment)?;
    update_database(&order, &payment)?;
    Ok(create_receipt(order, payment))
}

2. Primitive Obsession

// ❌ Code Smell: Using primitives for domain concepts
fn transfer_money(
    from_account: String,
    to_account: String,
    amount: f64,
    currency: String,
) -> Result<(), Error> {
    // Easy to mix up parameters
}

// ✅ Refactored: Domain types
#[derive(Debug, Clone)]
struct AccountId(String);

#[derive(Debug, Clone)]
struct Money {
    amount: Decimal,
    currency: Currency,
}

fn transfer_money(
    from: AccountId,
    to: AccountId,
    amount: Money,
) -> Result<(), Error> {
    // Type safety prevents mistakes
}

3. Feature Envy

// ❌ Code Smell: Method uses another object's data excessively
impl ShoppingCart {
    fn calculate_discount(&self, customer: &Customer) -> Money {
        let base_discount = if customer.loyalty_points > 1000 {
            0.10
        } else if customer.loyalty_points > 500 {
            0.05
        } else {
            0.0
        };
        
        let vip_bonus = if customer.is_vip {
            0.05
        } else {
            0.0
        };
        
        // Using customer's data more than own
    }
}

// ✅ Refactored: Move to where the data lives
impl Customer {
    fn calculate_discount_rate(&self) -> f64 {
        let base = match self.loyalty_points {
            p if p > 1000 => 0.10,
            p if p > 500 => 0.05,
            _ => 0.0,
        };
        
        if self.is_vip { base + 0.05 } else { base }
    }
}

impl ShoppingCart {
    fn calculate_discount(&self, customer: &Customer) -> Money {
        self.total * customer.calculate_discount_rate()
    }
}

4. Data Clumps

// ❌ Code Smell: Same parameters repeatedly appear together
fn create_address(street: String, city: String, zip: String, country: String) { }
fn validate_address(street: &str, city: &str, zip: &str, country: &str) -> bool { }
fn format_address(street: &str, city: &str, zip: &str, country: &str) -> String { }

// ✅ Refactored: Group into a type
#[derive(Debug, Clone)]
struct Address {
    street: String,
    city: String,
    zip: String,
    country: String,
}

fn create_address(address: Address) { }
fn validate_address(address: &Address) -> bool { }
fn format_address(address: &Address) -> String { }

---

Naming for Maximum Clarity

The Naming Hierarchy

Level 1: Terrible (Misleading)

fn calculate_total(items: Vec<Item>) -> f64 {
    // Actually calculates average, not total!
    items.iter().map(|i| i.price).sum::<f64>() / items.len() as f64
}

Level 2: Bad (Cryptic)

fn ct(i: Vec<Item>) -> f64 { }  // What does 'ct' mean?

Level 3: Acceptable (Generic)

fn process_items(items: Vec<Item>) -> f64 { }  // Process how?

Level 4: Good (Descriptive)

fn calculate_average_price(items: Vec<Item>) -> f64 { }

Level 5: Excellent (Self-Documenting)

fn calculate_average_price_excluding_tax(items: &[Item]) -> Money { }

Naming Patterns by Context

Boolean Variables/Functions

// ✅ Use is_, has_, can_, should_
is_valid, has_children, can_edit, should_retry

// ❌ Avoid ambiguous names
flag, check, status

Functions That Return Values

// ✅ Use get_, find_, calculate_, build_
get_user_by_id, find_first_match, calculate_tax, build_response

// ❌ Avoid vague verbs
do_thing, handle_stuff, process_data

Functions That Modify State

// ✅ Use set_, update_, add_, remove_, clear_
set_password, update_inventory, add_item, remove_expired, clear_cache

// ❌ Avoid ambiguous verbs
change, modify, alter

Collections

// ✅ Pluralize or use _list, _map, _set suffixes
users, active_users, user_ids, email_to_user_map

// ❌ Avoid singular names for collections
user, data, item

---

Extract Method Refactoring Pattern

Before: Complex Nested Logic

// ❌ Hard to understand, test, and modify
fn process_payment(order: &Order) -> Result<Payment, Error> {
    // Inline validation
    if order.items.is_empty() {
        return Err(Error::EmptyOrder);
    }
    
    let mut total = 0.0;
    for item in &order.items {
        if item.quantity <= 0 {
            return Err(Error::InvalidQuantity);
        }
        total += item.price * item.quantity as f64;
    }
    
    // Inline discount calculation
    let discount = if order.customer.is_vip {
        if total > 1000.0 {
            total * 0.20
        } else {
            total * 0.10
        }
    } else if total > 500.0 {
        total * 0.05
    } else {
        0.0
    };
    
    let final_amount = total - discount;
    
    // Inline payment processing
    // ... 20 more lines
}

After: Extracted Methods

// ✅ Clear, testable, modifiable
fn process_payment(order: &Order) -> Result<Payment, Error> {
    validate_order(order)?;
    let total = calculate_order_total(order)?;
    let discount = calculate_discount(total, &order.customer);
    let final_amount = total - discount;
    
    charge_payment(final_amount, &order.payment_method)
}

fn validate_order(order: &Order) -> Result<(), Error> {
    if order.items.is_empty() {
        return Err(Error::EmptyOrder);
    }
    
    for item in &order.items {
        validate_item(item)?;
    }
    
    Ok(())
}

fn validate_item(item: &OrderItem) -> Result<(), Error> {
    if item.quantity <= 0 {
        return Err(Error::InvalidQuantity);
    }
    Ok(())
}

fn calculate_order_total(order: &Order) -> Result<Money, Error> {
    Ok(order.items
        .iter()
        .map(|item| item.price * item.quantity as f64)
        .sum())
}

fn calculate_discount(total: Money, customer: &Customer) -> Money {
    match (customer.is_vip, total.amount) {
        (true, amount) if amount > 1000.0 => total * 0.20,
        (true, _) => total * 0.10,
        (false, amount) if amount > 500.0 => total * 0.05,
        _ => Money::zero(),
    }
}

---

Replace Conditional with Polymorphism

Before: Switch/Match Explosion

// ❌ Adding new types requires modifying multiple functions
enum UserType {
    Free,
    Premium,
    Enterprise,
}

fn get_storage_limit(user_type: &UserType) -> u64 {
    match user_type {
        UserType::Free => 1_000_000_000,        // 1GB
        UserType::Premium => 100_000_000_000,   // 100GB
        UserType::Enterprise => 1_000_000_000_000, // 1TB
    }
}

fn get_api_rate_limit(user_type: &UserType) -> u32 {
    match user_type {
        UserType::Free => 100,
        UserType::Premium => 1000,
        UserType::Enterprise => 10000,
    }
}

fn can_access_feature(user_type: &UserType, feature: &str) -> bool {
    match user_type {
        UserType::Free => matches!(feature, "basic_search"),
        UserType::Premium => !matches!(feature, "enterprise_analytics"),
        UserType::Enterprise => true,
    }
}

After: Trait-Based Polymorphism

// ✅ Adding new types only requires implementing the trait
trait UserTier {
    fn storage_limit(&self) -> u64;
    fn api_rate_limit(&self) -> u32;
    fn can_access_feature(&self, feature: &str) -> bool;
}

struct FreeUser;
impl UserTier for FreeUser {
    fn storage_limit(&self) -> u64 { 1_000_000_000 }
    fn api_rate_limit(&self) -> u32 { 100 }
    fn can_access_feature(&self, feature: &str) -> bool {
        matches!(feature, "basic_search")
    }
}

struct PremiumUser;
impl UserTier for PremiumUser {
    fn storage_limit(&self) -> u64 { 100_000_000_000 }
    fn api_rate_limit(&self) -> u32 { 1000 }
    fn can_access_feature(&self, feature: &str) -> bool {
        !matches!(feature, "enterprise_analytics")
    }
}

struct EnterpriseUser;
impl UserTier for EnterpriseUser {
    fn storage_limit(&self) -> u64 { 1_000_000_000_000 }
    fn api_rate_limit(&self) -> u32 { 10000 }
    fn can_access_feature(&self, _feature: &str) -> bool { true }
}

---

Introduce Parameter Object

Before: Too Many Parameters

// ❌ Hard to remember parameter order, easy to make mistakes
fn create_user(
    first_name: String,
    last_name: String,
    email: String,
    phone: String,
    address_line1: String,
    address_line2: Option<String>,
    city: String,
    state: String,
    zip: String,
    country: String,
    newsletter: bool,
    terms_accepted: bool,
) -> Result<User, Error> {
    // Function body
}

After: Parameter Object

// ✅ Clear, extensible, impossible to mix up parameters
#[derive(Debug, Default, Builder)]
struct CreateUserRequest {
    personal_info: PersonalInfo,
    address: Address,
    preferences: UserPreferences,
}

#[derive(Debug, Default)]
struct PersonalInfo {
    first_name: String,
    last_name: String,
    email: String,
    phone: String,
}

#[derive(Debug, Default)]
struct Address {
    line1: String,
    line2: Option<String>,
    city: String,
    state: String,
    zip: String,
    country: String,
}

#[derive(Debug, Default)]
struct UserPreferences {
    newsletter: bool,
    terms_accepted: bool,
}

fn create_user(request: CreateUserRequest) -> Result<User, Error> {
    validate_personal_info(&request.personal_info)?;
    validate_address(&request.address)?;
    validate_preferences(&request.preferences)?;
    
    // Create user
}

// Usage with builder pattern
let user = create_user(
    CreateUserRequest::builder()
        .personal_info(personal_info)
        .address(address)
        .preferences(preferences)
        .build()
)?;

---

Replace Magic Numbers with Named Constants

Before: Magic Numbers Everywhere

// ❌ What do these numbers mean?
fn calculate_shipping(weight: f64, distance: f64) -> f64 {
    if weight > 50.0 {
        return distance * 0.75 + 20.0;
    }
    
    if distance > 1000.0 {
        return weight * 0.5 + distance * 0.02;
    }
    
    weight * 0.3 + distance * 0.01
}

After: Self-Documenting Constants

// ✅ Clear intent and business rules
const HEAVY_PACKAGE_THRESHOLD_KG: f64 = 50.0;
const LONG_DISTANCE_THRESHOLD_KM: f64 = 1000.0;

const HEAVY_PACKAGE_RATE_PER_KM: f64 = 0.75;
const HEAVY_PACKAGE_BASE_FEE: f64 = 20.0;

const LONG_DISTANCE_WEIGHT_RATE: f64 = 0.5;
const LONG_DISTANCE_KM_RATE: f64 = 0.02;

const STANDARD_WEIGHT_RATE: f64 = 0.3;
const STANDARD_KM_RATE: f64 = 0.01;

fn calculate_shipping(weight_kg: f64, distance_km: f64) -> f64 {
    if weight_kg > HEAVY_PACKAGE_THRESHOLD_KG {
        return distance_km * HEAVY_PACKAGE_RATE_PER_KM + HEAVY_PACKAGE_BASE_FEE;
    }
    
    if distance_km > LONG_DISTANCE_THRESHOLD_KM {
        return weight_kg * LONG_DISTANCE_WEIGHT_RATE 
             + distance_km * LONG_DISTANCE_KM_RATE;
    }
    
    weight_kg * STANDARD_WEIGHT_RATE + distance_km * STANDARD_KM_RATE
}

---

Early Return Pattern

Before: Nested Conditionals

// ❌ Arrow anti-pattern, hard to follow flow
fn process_request(request: Request) -> Result<Response, Error> {
    if request.is_valid() {
        if request.user.is_authenticated() {
            if request.user.has_permission() {
                if !rate_limiter.is_exceeded(&request.user) {
                    // Actual processing
                    let result = do_work(request);
                    Ok(result)
                } else {
                    Err(Error::RateLimitExceeded)
                }
            } else {
                Err(Error::Unauthorized)
            }
        } else {
            Err(Error::NotAuthenticated)
        }
    } else {
        Err(Error::InvalidRequest)
    }
}

After: Guard Clauses

// ✅ Linear flow, easy to understand
fn process_request(request: Request) -> Result<Response, Error> {
    if !request.is_valid() {
        return Err(Error::InvalidRequest);
    }
    
    if !request.user.is_authenticated() {
        return Err(Error::NotAuthenticated);
    }
    
    if !request.user.has_permission() {
        return Err(Error::Unauthorized);
    }
    
    if rate_limiter.is_exceeded(&request.user) {
        return Err(Error::RateLimitExceeded);
    }
    
    // Happy path is clear and unindented
    let result = do_work(request);
    Ok(result)
}

---

Comment Refactoring

Types of Comments to Remove

1. Redundant Comments

// ❌ Comments that repeat the code
let count = 0; // Set count to 0
for item in items { // Loop through items
    count += 1; // Increment count
}

// ✅ Self-explanatory code needs no comments
let count = items.len();

2. Commented-Out Code

// ❌ Version control exists for a reason
fn calculate_total(items: &[Item]) -> f64 {
    // let old_total = 0.0;
    // for item in items {
    //     old_total += item.price;
    // }
    
    items.iter().map(|i| i.price).sum()
}

// ✅ Delete it. Git remembers everything
fn calculate_total(items: &[Item]) -> f64 {
    items.iter().map(|i| i.price).sum()
}

3. Excuse Comments

// ❌ Comments explaining bad code
// This is a bit hacky but it works
// TODO: Fix this later
// Sorry for the mess

// ✅ Fix the code instead of apologizing

Comments Worth Keeping

1. Why Comments (Not What)

// ✅ Explains business logic/decisions
// We use 1.5x rate for overtime because of union agreement dated 2023-01-15
const OVERTIME_MULTIPLIER: f64 = 1.5;

// Apply Gaussian blur with sigma=2.0 to reduce noise while preserving edges
// Research paper: https://example.com/noise-reduction.pdf
image.gaussian_blur(2.0);

2. Warning Comments

// ⚠️ CRITICAL: This order matters! Authentication must happen before authorization
authenticate(&user)?;
authorize(&user, &resource)?;

// ⚠️ PERFORMANCE: This query scans the entire table. Only use for daily batch jobs
let all_users = sqlx::query("SELECT * FROM users").fetch_all(&pool).await?;

3. Legal/License Comments

// Copyright (c) 2024 Company Name
// SPDX-License-Identifier: MIT

---

Code Formatting for Readability

Vertical Spacing

// ❌ Wall of text
fn process_order(order: Order) -> Result<Receipt, Error> {
    validate_order(&order)?;
    let items = order.items.clone();
    let customer = get_customer(order.customer_id)?;
    let discount = calculate_discount(&customer);
    let subtotal = calculate_subtotal(&items);
    let tax = calculate_tax(subtotal);
    let total = subtotal - discount + tax;
    charge_payment(&customer, total)?;
    update_inventory(&items)?;
    send_email(&customer, &order)?;
    Ok(create_receipt(order, total))
}

// ✅ Logical grouping with whitespace
fn process_order(order: Order) -> Result<Receipt, Error> {
    // Validation
    validate_order(&order)?;
    
    // Gather data
    let items = order.items.clone();
    let customer = get_customer(order.customer_id)?;
    
    // Calculate pricing
    let discount = calculate_discount(&customer);
    let subtotal = calculate_subtotal(&items);
    let tax = calculate_tax(subtotal);
    let total = subtotal - discount + tax;
    
    // Execute transaction
    charge_payment(&customer, total)?;
    update_inventory(&items)?;
    
    // Post-processing
    send_email(&customer, &order)?;
    Ok(create_receipt(order, total))
}

Alignment for Clarity

// ❌ Hard to scan
let config = Config {
    host: "localhost",
    port: 8080,
    timeout_ms: 5000,
    max_connections: 100,
    enable_tls: true,
};

// ✅ Aligned for easy scanning
let config = Config {
    host:            "localhost",
    port:            8080,
    timeout_ms:      5000,
    max_connections: 100,
    enable_tls:      true,
};

// Or with longer names, consider grouping
let config = Config {
    // Network settings
    host: "localhost",
    port: 8080,
    
    // Performance settings
    timeout_ms: 5000,
    max_connections: 100,
    
    // Security settings
    enable_tls: true,
};

---

Refactoring Checklist

Before Starting Refactoring

• Tests exist - Never refactor without tests
• Tests pass - Ensure current behavior is captured
• Commit current state - Can revert if needed
• Understand the code - Know what it does and why
• Identify the smell - Know what you're fixing

During Refactoring

• Small steps - One refactoring at a time
• Run tests frequently - After each small change
• Keep it working - Never break the build
• Preserve behavior - Refactoring doesn't change functionality
• Update tests - If interface changes

After Refactoring

• All tests pass - Behavior preserved
• Code is cleaner - Measurably improved
• Documentation updated - If public API changed
• Team review - Get feedback on improvements
• Performance checked - No regressions

Common Refactoring Patterns Priority

1. Extract Function - Most common, highest impact
2. Rename - Improves understanding immediately
3. Extract Variable - Clarifies complex expressions
4. Inline Variable - Removes unnecessary indirection
5. Replace Magic Number - Adds semantic meaning
6. Introduce Parameter Object - Simplifies signatures
7. Replace Conditional with Polymorphism - For type-based logic
8. Extract Class/Module - For cohesion
9. Move Function - Put code where it belongs
10. Replace Loop with Pipeline - Functional transformation

---

Readability Review Checklist

Naming

• Functions describe what they do
• Variables describe what they contain
• No abbreviations or acronyms
• Consistent terminology throughout
• Domain language used correctly

Structure

• Functions under 40 lines
• Maximum 3 levels of nesting
• Clear separation of concerns
• Logical grouping with whitespace
• Related code is together

Complexity

• Cyclomatic complexity ≤10
• No clever tricks or golf code
• Obvious over optimal (unless critical path)
• Complex logic has explanatory variables
• No premature optimization

Documentation

• Public APIs documented
• Complex algorithms explained
• Business rules noted
• Assumptions stated
• Examples provided

Consistency

• Follows team style guide
• Pattern usage is consistent
• Error handling uniform
• Similar problems solved similarly
• No surprising behavior

---

Critical Review Checklist

For Every Review:

• Every line examined - No skimming
• Clippy clean - All warnings addressed
• SQL patterns checked - No N+1, proper indexes
• Error handling complete - All Results handled
• Tests included - Business logic covered
• Documentation updated - Public APIs documented
• Performance considered - No obvious bottlenecks
• Security validated - Input sanitization, SQL parameterization
• Concurrency safe - No data races or deadlocks
• Future-proof - Easy to modify and extend

Before Approving:

• Does this improve overall system health?
• Are all critical issues addressed?
• Is the code better than before?
• Will this be maintainable in 6 months?
• Have I provided educational feedback?

---

SQL Query Review Checklist

For Every Query:

• Parameterized - No string concatenation
• Index usage - EXPLAIN QUERY PLAN reviewed
• Column specification - No SELECT *
• Join type correct - INNER vs LEFT considered
• NULL handling - IS NULL/IS NOT NULL used correctly
• Type matching - No implicit conversions
• Aggregation correct - GROUP BY includes all non-aggregated columns
• DISTINCT justified - Not covering bad JOINs
• Pagination included - LIMIT for large results
• Transaction scope - Appropriate BEGIN/COMMIT placement

For Complex Queries:

• CTEs used - Instead of nested subqueries
• Window functions - Properly partitioned
• UNION ALL - Used instead of UNION when possible
• Covering indexes - Include all needed columns
• Statistics updated - ANALYZE run recently

---

Merge Conflict Prevention

Code Structure for Minimal Conflicts

1. Vertical Organization

// ✅ Merge-friendly
use crate::user::User;
use crate::order::Order;
use crate::payment::Payment;

const USER_TIMEOUT: u64 = 30;
const ORDER_TIMEOUT: u64 = 60;
const PAYMENT_TIMEOUT: u64 = 90;

// ❌ Conflict-prone
use crate::{user::User, order::Order, payment::Payment};
const USER_TIMEOUT: u64 = 30, ORDER_TIMEOUT: u64 = 60;

2. Append-Only Patterns

// ✅ New items at end
enum Status {
    Active,
    Inactive,
    Deleted,
    // New variants go here
    Pending,    // Added by dev 1
    Processing, // Added by dev 2 - No conflict!
}

3. Trailing Commas Always

// ✅ Prevents conflicts
let config = Config {
    host: "localhost",
    port: 8080,  // Trailing comma
    // New fields can be added without touching previous line
}

---

Review Response Templates

For Critical Issues:

🚨 **BLOCKING: Security Vulnerability**

SQL injection vulnerability in line 45. User input is concatenated 
directly into the query string.

**Required Fix**:
```rust
// Replace this:
let query = format!("SELECT * FROM users WHERE id = {}", user_id);

// With:
sqlx::query("SELECT * FROM users WHERE id = ?")
    .bind(user_id)

This must be fixed before merge as it exposes the database to injection attacks. See OWASP guide: [link]

### For Performance Issues:
```markdown
⚠️ **Performance: N+1 Query Pattern**

The loop starting at line 78 executes a query for each user, 
resulting in N+1 database calls.

**Suggested Optimization**:
Consider using a JOIN or IN clause to fetch all data in one query.
This could reduce database round-trips from 100+ to 1.

Example refactor in our patterns guide: [link]

*Impact*: High - This is in a hot path called on every page load.

For Good Practices:

👍 **Excellent Error Handling**

Great use of the `?` operator and custom error types here. The 
error messages are descriptive and will help with debugging.

One minor suggestion: Consider adding error context with `anyhow::Context`:
```rust
.with_context(|| format!("Failed to process user {}", user_id))?

---

## Conclusion

This comprehensive guide integrates Google's review philosophy, Rust's official guidelines, SQLite's performance characteristics, and systematic detection strategies. Success depends on:

1. **Layered Detection**: Automated tools catch mechanical issues, humans evaluate design
2. **Time-Boxed Reviews**: Respond within one business day, use "LGTM with comments"
3. **Educational Focus**: Explain why, not just what to change
4. **Systematic Approach**: Use checklists, maintain pattern catalogs
5. **Continuous Improvement**: Track metrics, update guidelines

Remember: **Approve changes that improve the codebase, even if imperfect**. Perfect code doesn't exist—only better code does.