Skip to content

EXAMPLES.md

Latest commit

 

History

History
447 lines (330 loc) · 10.3 KB

EXAMPLES.md

File metadata and controls

447 lines (330 loc) · 10.3 KB

Billy Bullshit - Code Review Examples

Billy's PRIMARY function is to call out BS in your code. Here's what that looks like in action.

Quick Reference: BS Categories

Billy classifies code issues into 4 categories:

  • 🚩 CRITICAL - Security holes, data loss, crashes waiting to happen
  • ⚠️ MAJOR - Performance nightmares, maintainability disasters
  • 💩 BS - Over-engineering, cargo culting, bad practices
  • 🤦 WTAF - Code that makes you question humanity

Every review includes a BS Score (1-10) where 10 = complete disaster.

Example 1: Redundant Boolean Logic

Your Code

function isUserActive(user) {
  if (user.active == true) {
    return true;
  } else {
    return false;
  }
}

Billy's Review

BS SCORE: 8/10

🚩 CRITICAL ISSUES: None. At least it won't crash.

⚠️ MAJOR ISSUES: None. But the BS is strong.

💩 BS DETECTOR: Just return the fucking condition. One line. You wrote 5 to do what 1 does.

  • if (user.active == true) - This is redundant. user.active is already a boolean.
  • { return true; } else { return false; } - You're literally writing 5 lines to return a boolean.

THE FIX:

function isUserActive(user) {
  return user.active;
}

One. Line. That's all you need. Stop over-complicating simple logic.

Example 2: Cargo Cult Naming

Your Code

class UserFactoryManagerHelperUtil {
  constructor(private userServiceProviderFactory: UserServiceProviderFactory) {}

  public createUserInstanceObject(userData: any): any {
    return this.userServiceProviderFactory.getInstance().createUser(userData);
  }
}

Billy's Review

BS SCORE: 9/10

🚩 CRITICAL ISSUES:

  • any types everywhere. TypeScript without types is just JavaScript with extra steps.

⚠️ MAJOR ISSUES: None that matter compared to the naming disaster.

💩 BS DETECTOR:

  • UserFactoryManagerHelperUtil - You've combined EVERY bad naming convention into one abomination. Factory? Manager? Helper? Util? Pick ONE.
  • createUserInstanceObject - "Instance" and "Object" add ZERO information. It's a function that creates a user. Call it createUser.
  • UserServiceProviderFactory - More buzzword bingo. What does this actually DO?

🤦 WTAF: You're writing Java in TypeScript. Stop it.

THE FIX:

class UserRepository {
  constructor(private db: Database) {}

  create(data: CreateUserDto): User {
    return this.db.users.create(data);
  }
}

Simple. Clear. Descriptive. No buzzwords.

Example 3: Error Swallowing

Your Code

def process_payment(amount, user_id):
    try:
        charge_credit_card(amount, user_id)
        send_confirmation_email(user_id)
        update_database(user_id, amount)
    except:
        pass

Billy's Review

BS SCORE: 10/10 🔥

🚩 CRITICAL ISSUES:

  • Empty except block on a PAYMENT function? Are you fucking kidding me?
  • When this fails (and it will), you'll have NO IDEA what happened
  • User gets charged but no confirmation? Or confirmation but no charge? WHO KNOWS!
  • You're silently eating payment errors. This is a lawsuit waiting to happen.

⚠️ MAJOR ISSUES:

  • No logging, no monitoring, no alerting
  • No transaction rollback
  • No way to debug when (not if) this breaks

💩 BS DETECTOR: You copy-pasted this from somewhere and added the try/except to "fix" an error.

THE FIX:

import logging

def process_payment(amount: float, user_id: str) -> bool:
    try:
        charge_id = charge_credit_card(amount, user_id)
        send_confirmation_email(user_id, charge_id)
        update_database(user_id, amount, charge_id)
        return True
    except PaymentError as e:
        logging.error(f"Payment failed for user {user_id}: {e}")
        # Rollback any partial charges
        refund_if_charged(charge_id)
        raise  # Re-raise so caller knows it failed
    except Exception as e:
        logging.critical(f"Unexpected payment error for user {user_id}: {e}")
        raise

AT MINIMUM: log the error. Better: handle it properly with rollbacks and alerts.

Example 4: Useless Comments

Your Code

// This function adds two numbers
function add(a, b) {
  return a + b;
}

// Loop through the array
for (let i = 0; i < users.length; i++) {
  // Get the user
  const user = users[i];
  // Check if user is active
  if (user.active) {
    // Process the user
    processUser(user);
  }
}

Billy's Review

BS SCORE: 4/10

🚩 CRITICAL ISSUES: None.

⚠️ MAJOR ISSUES: None.

💩 BS DETECTOR: Your comments explain WHAT the code does. I can fucking read code. Comments should explain WHY.

  • "This function adds two numbers" - No shit. It's called add.
  • "Loop through the array" - I can see the for loop.
  • "Get the user" - Thanks, I thought we were getting a pizza.
  • "Check if user is active" - Wow, really helpful.

Every single comment is USELESS.

THE FIX:

// Calculate total from two partial payments
function add(a, b) {
  return a + b;
}

// Process only active users to avoid hitting disabled accounts
// that would trigger rate limit errors
for (const user of users) {
  if (user.active) {
    processUser(user);
  }
}

Comments should explain WHY you're doing something, not WHAT you're doing. The code already shows WHAT.

Also, use for...of instead of that ancient index loop.

Example 5: Nested Ternary Hell

Your Code

const status = user.active ?
  user.verified ?
    user.premium ? 'premium-active' : 'active' :
  'unverified' :
  'inactive';

Billy's Review

BS SCORE: 7/10

🚩 CRITICAL ISSUES: None. But good luck debugging this.

⚠️ MAJOR ISSUES:

  • Unreadable
  • Hard to test
  • Error-prone
  • Makes code reviewers cry

💩 BS DETECTOR: You thought this looked "clean" and "concise". It's not. It's a fucking nightmare.

Nested ternaries are code golf, not production code.

THE FIX:

function getUserStatus(user) {
  if (!user.active) return 'inactive';
  if (!user.verified) return 'unverified';
  return user.premium ? 'premium-active' : 'active';
}

const status = getUserStatus(user);

Look at that. Readable. Testable. Debuggable. Revolutionary concepts.

Example 6: Premature Optimization

Your Code

// Ultra-optimized user lookup with caching and memoization
class UserLookupOptimizer {
  private cache = new Map<string, WeakMap<any, User>>();
  private memoized = new WeakMap<Function, Map<string, any>>();

  async getUser(id: string): Promise<User> {
    // 50 lines of caching logic for 10 users
    // ...
  }
}

Billy's Review

BS SCORE: 9/10

🚩 CRITICAL ISSUES:

  • WeakMap usage makes no sense here
  • Memory leaks waiting to happen

⚠️ MAJOR ISSUES:

  • Complexity explosion for ZERO benefit
  • You have 10 users. TEN.
  • You don't need this.

💩 BS DETECTOR: You read a blog post about optimization and went nuts. Classic premature optimization.

"Premature optimization is the root of all evil" - Donald Knuth

You have 10 users. A simple object would work fine. When you have 10 MILLION users, THEN optimize.

THE FIX:

class UserRepository {
  async getUser(id: string): Promise<User> {
    return await db.users.findById(id);
  }
}

Simple. Works. When you actually NEED caching (you don't), THEN add it.

Example 7: God Function

Your Code

def handle_user_request(request, user_id, action, data, options, context):
    # Validate user
    # Check permissions
    # Parse data
    # Validate data
    # Update database
    # Send email
    # Log action
    # Update cache
    # Trigger webhooks
    # Generate analytics
    # Return response
    # ... 300 lines later

Billy's Review

BS SCORE: 8/10

🚩 CRITICAL ISSUES:

  • Impossible to test
  • Impossible to debug
  • Does too many things

⚠️ MAJOR ISSUES:

  • Single Responsibility Principle? Never heard of it.
  • 300 lines in one function
  • Probably has 10+ responsibilities

💩 BS DETECTOR: This function does EVERYTHING. It's the Swiss Army knife of functions. And just like a Swiss Army knife, it's shit at most of what it does.

THE FIX:

def handle_user_request(request: Request, user_id: str, action: str) -> Response:
    user = validate_and_get_user(user_id)
    check_permissions(user, action)

    data = parse_and_validate_data(request.data)
    result = perform_action(user, action, data)

    send_notifications(user, action, result)
    log_and_track(user, action, result)

    return create_response(result)

Each function does ONE thing. Easy to test. Easy to debug. Easy to maintain.

Example 8: Magic Numbers

Your Code

if (user.loginAttempts > 5) {
  lockAccount(user, 1800);
}

setTimeout(cleanupOldData, 86400000);

const discount = price * 0.15;

Billy's Review

BS SCORE: 6/10

🚩 CRITICAL ISSUES: None.

⚠️ MAJOR ISSUES:

  • Good luck remembering what 1800, 86400000, or 0.15 mean in 6 months

💩 BS DETECTOR: Magic numbers everywhere. What's 1800? Seconds? Minutes? Why 5 attempts? Why not 3 or 10?

In 6 months you'll come back to this code and have NO IDEA what these numbers mean.

THE FIX:

const MAX_LOGIN_ATTEMPTS = 5;
const ACCOUNT_LOCK_DURATION_SECONDS = 30 * 60; // 30 minutes
const CLEANUP_INTERVAL_MS = 24 * 60 * 60 * 1000; // 24 hours
const LOYALTY_DISCOUNT_RATE = 0.15; // 15% off

if (user.loginAttempts > MAX_LOGIN_ATTEMPTS) {
  lockAccount(user, ACCOUNT_LOCK_DURATION_SECONDS);
}

setTimeout(cleanupOldData, CLEANUP_INTERVAL_MS);

const discount = price * LOYALTY_DISCOUNT_RATE;

Named constants. Self-documenting. Maintainable. You're welcome.

How to Use These Examples

  1. Learn from them - See what Billy looks for
  2. Compare your code - Does your code have similar BS?
  3. Fix it - Apply the same principles
  4. Submit for review - Let Billy call out what you missed

API Usage

curl -X POST https://billy.chitty.cc/review \
  -H "Content-Type: application/json" \
  -d '{
    "code": "your code here",
    "language": "javascript",
    "context": "optional context"
  }'

Billy will return a structured review with:

  • BS Score (1-10)
  • Critical issues (🚩)
  • Major issues (⚠️)
  • BS detected (💩)
  • WTAF moments (🤦)
  • The fix

Tagline: Calling BS on your BS code since 2024 💩