Code Smells & Refactoring

Stage 3 · Testing, Quality & Craft · B.U.I.L.D. letter: U

You shipped it. It works. But every time you open that file, something in your gut says "I should not have to think this hard to understand my own code." That feeling has a name — and a fix.

---

⚠️ The vibe trap

You've probably heard "refactoring" used loosely to mean "I rewrote some stuff." That's not refactoring — that's a rewrite, and rewrites break things. Real refactoring has one iron rule: the behavior of the program does not change, only its structure. The other trap is the opposite: you vibe-coded something fast, it shipped, and now nobody ever touches it because the code is too tangled to read safely. Technical debt doesn't charge interest monthly — it charges it every single time a teammate tries to extend your work. Cleaning up is not an indulgence; it is part of your job.

---

🦨 Recognizing Code Smells

A code smell is not a bug. It is a surface signal that the structure of your code is working against you. You learn to spot them by feel first, then by name.

The six you'll encounter most:

You don't need to memorize the list. You need to feel the friction and recognize it as "this is a smell, not bad luck."

Common mistake: Trying to fix all smells in one sitting. Pick one, fix it, commit, move on.

---

✂️ Extract Function — When One Thing Does Too Many Things

The most-used refactoring move. When a block of code inside a function has a clear purpose you can name, pull it out.

Before — one function doing three jobs:

function sendWelcomeEmail(user) {
  // build greeting
  let greeting;
  if (user.firstName) {
    greeting = `Hi ${user.firstName},`;
  } else {
    greeting = "Hi there,";
  }

  // build body
  const body = `${greeting}\n\nWelcome to HYVE. Your account is ready.\n\nLearn something today!`;

  // send
  fetch("/api/email", {
    method: "POST",
    headers: { "Content-Type": "application/json" },
    body: JSON.stringify({ to: user.email, subject: "Welcome!", body }),
  });
}

After — each piece has a name:

function buildGreeting(user) {
  return user.firstName ? `Hi ${user.firstName},` : "Hi there,";
}

function buildWelcomeBody(user) {
  const greeting = buildGreeting(user);
  return `${greeting}\n\nWelcome to HYVE. Your account is ready.\n\nLearn something today!`;
}

function sendWelcomeEmail(user) {
  const body = buildWelcomeBody(user);
  return fetch("/api/email", {
    method: "POST",
    headers: { "Content-Type": "application/json" },
    body: JSON.stringify({ to: user.email, subject: "Welcome!", body }),
  });
}

Mental model: If you need a comment to explain what a block of code does, that comment is the name of the function you should extract.

Why it matters: Each extracted function is now independently testable. buildGreeting has zero side effects and four possible inputs — you can test it in two lines.

Common mistake: Extracting so aggressively that every two-line helper gets its own function. Extract when the piece has a clear name and a clear reason to exist on its own.

---

🔢 Replace Magic Number / Magic String with a Named Constant

Raw literals scattered through code are lies waiting to happen. The number 7 means nothing. TRIAL_DAYS means everything.

Before:

function isTrialExpired(user) {
  const msPerDay = 1000 * 60 * 60 * 24;
  const daysSinceSignup = (Date.now() - user.createdAt) / msPerDay;
  return daysSinceSignup > 7;
}

function trialBadgeLabel(user) {
  const msPerDay = 1000 * 60 * 60 * 24;
  const daysLeft = 7 - (Date.now() - user.createdAt) / msPerDay;
  return daysLeft > 0 ? `${Math.ceil(daysLeft)} days left` : "Trial ended";
}

After:

const TRIAL_DAYS = 7;
const MS_PER_DAY = 1000 * 60 * 60 * 24;

function daysSince(timestamp) {
  return (Date.now() - timestamp) / MS_PER_DAY;
}

function isTrialExpired(user) {
  return daysSince(user.createdAt) > TRIAL_DAYS;
}

function trialBadgeLabel(user) {
  const daysLeft = TRIAL_DAYS - daysSince(user.createdAt);
  return daysLeft > 0 ? `${Math.ceil(daysLeft)} days left` : "Trial ended";
}

Mental model: Every magic literal is a question your reader has to answer: "Why 7? Is that the same 7 as the other one?" A named constant removes the question permanently.

Why it matters: When the trial period changes to 14 days, you change one line, not a grep-and-pray hunt across five files.

Common mistake: Creating a constant for a value that genuinely is a one-off and will never be reused or changed. Not every literal needs a name — only the ones that carry business meaning.

---

🗺️ Replace Big Conditional with a Lookup Table

When a conditional's only job is to map one value to another, a data structure beats branching code every time.

Before — a conditional that will grow forever:

function getBadgeColor(role) {
  if (role === "admin") {
    return "#FF4136";
  } else if (role === "teacher") {
    return "#2ECC40";
  } else if (role === "student") {
    return "#0074D9";
  } else if (role === "parent") {
    return "#FF851B";
  } else {
    return "#AAAAAA";
  }
}

After — a lookup table + a clean fallback:

const BADGE_COLORS = {
  admin:   "#FF4136",
  teacher: "#2ECC40",
  student: "#0074D9",
  parent:  "#FF851B",
};

const DEFAULT_BADGE_COLOR = "#AAAAAA";

function getBadgeColor(role) {
  return BADGE_COLORS[role] ?? DEFAULT_BADGE_COLOR;
}

Mental model: A switch or if/else if chain is a data structure pretending to be logic. When you need to add a new role, you add one line to an object — you do not open a function and add an else if branch that could accidentally fall through.

Why it matters: The lookup table is data. You can export it, test it directly, render it in a UI, or generate it from a database. The if/else chain cannot do any of those things.

Common mistake: Reaching for a lookup table when the branches have actual logic differences (not just a mapped value). If each branch does something meaningfully different, keep the conditional — or replace it with polymorphism/strategy objects.

---

🛡️ The Golden Rule: Refactor Under Green Tests

Every refactoring in this lesson assumed something: your tests are passing before you start, and they are still passing when you finish. That is not optional. It is the definition.

The workflow is always:

1. Run your tests. They must be green.
2. Make one small structural change (extract a function, rename a variable, pull out a constant).
3. Run your tests again. Still green? Commit.
4. Repeat.

Small steps feel slow until the day you make a large step, break something subtle, and spend three hours bisecting a diff you cannot read. Then small steps feel like wisdom.

When NOT to refactor:

• You have no tests covering the code you want to touch. Write tests first — or leave it alone.
• You are 30 minutes from a shipping deadline. Green code beats clean code today. Schedule a cleanup ticket.
• You are refactoring as a procrastination tactic to avoid a hard feature. Name the thing you're avoiding and go do it.

Common mistake: "I'll just quickly clean this up while I'm in here adding the feature." Now your diff mixes behavior changes with structural changes, your code reviewer cannot tell what changed, and if something breaks you cannot tell which part caused it. Refactors get their own commits.

---

🛠️ Your Mission

Open the project you've been building throughout this track. Find two code smells — at minimum one Long Function and one Big Conditional or magic literal. Before you touch anything, run your test suite and confirm it is green.

Then refactor each smell using the moves from this lesson:

1. Extract at least one function with a name that makes a comment unnecessary.
2. Replace at least one magic literal with a named constant, or replace a conditional with a lookup table.

After each change: run tests, confirm green, commit. Your commit message should say what you did, not what you changed — for example: refactor: extract buildGreeting from sendWelcomeEmail.

---

✅ You're done when…

• Your refactored code passes the Code Review Rubric — functions do one thing, names are intention-revealing, no magic literals, no commented-out code
• You have at least two commits: one per refactoring, each with a refactor: prefix in the message
• All tests were green before your first edit and are still green after your last
• You can explain to a teammate what each refactoring changed and why it is safer now

---

➡️ Next: Code Review Mastery.

Build It Right, Or Don't Build It At All. 🏛️