In this article
-
useEffect hiding what should be useMemo
-
try/catch as defect masking
-
Map objects replacing large if/else chains
-
Mode parameters that should be split functions
-
The cost of skipping utils and small functions
-
Boolean parameters as a smell
-
Nested conditionals vs early returns
-
Magic numbers and strings
-
Variable naming vs comments
-
Comments explain "why", not "what"
-
Avoid primitive obsession
-
Don't return different shapes from one function
-
Keep side effects separate from pure logic
01 — If you need useEffect to set state, you probably need useMemo
A useEffect that only sets state based on other state or props is a derived value wearing a disguise. Effects are for synchronizing with the outside world — DOM APIs, timers, subscriptions. When you see an effect that just transforms existing values and calls a setter, that's a signal: this is a computation, not a side effect.
"If it has no subscription, no network call, and no DOM mutation — it's probably useMemo."
smell
const [full, setFull] = useState('');
useEffect(() => {
setFull(`${first} ${last}`);
}, [first, last]);clean
const fullName = useMemo(
() => `${first} ${last}`,
[first, last]
);The effect version triggers an extra render cycle: state change → render → effect → state change → render again. useMemo computes once and only re-computes when dependencies change. Less re-rendering, less confusion, less code.
02 — try/catch is defect masking in disguise
There is a version of error handling that looks responsible but is actually hiding problems. It's the empty catch, or the catch that logs and silently continues, or the catch that returns a default value without telling anyone something went wrong.
smell
try {
const data = JSON.parse(input);
processData(data);
} catch (e) {
// ...
}clean
try {
const data = JSON.parse(input);
processData(data);
} catch (e) {
logger.error('Parse failed', {
input, error: e.message
});
throw new AppError('Invalid input', e);
}Error handling is not the same as error swallowing. A try/catch should be an active decision: log it, rethrow it, recover explicitly, or notify the user. If you can't articulate what happens when this fails, the catch block is just hiding a future bug report.
03 — Large if/else chains that a small map object can replace
Long chains of if (x === 'a') ... else if (x === 'b') ... are often just a lookup table written in the hardest way. When each branch returns a simple value or calls a known function, a map is cleaner, shorter, and easier to extend.
smell
if (status === 'pending') return '🕐';
else if (status === 'active') return '✅';
else if (status === 'paused') return '⏸';
else if (status === 'failed') return '❌';
else return '?';clean
const STATUS_ICON = {
pending: '🕐',
active: '✅',
paused: '⏸',
failed: '❌',
};
return STATUS_ICON[status] ?? '?';Map objects are data. They can be exported, tested independently, composed, and extended without touching control flow. Adding a new status is a one-line data change, not a surgical edit to a conditional tree. This pattern also works for mapping event names to handlers, role names to components, and route strings to config objects.
04 — A function that takes a "mode" probably needs to be split
When a function accepts a mode parameter and each mode makes the function do something fundamentally different, you don't have one function — you have two or three functions stapled together. The mode is a seam. Follow it.
smell
function processUser(user, mode) {
if (mode === 'create') {
// 15 lines of creation logic
} else if (mode === 'update') {
// 15 lines of update logic
} else if (mode === 'delete') {
// 10 lines of deletion logic
}
}clean
function createUser(user) { ... }
function updateUser(user) { ... }
function deleteUser(userId) { ... }Separate functions are separately testable, separately readable, and separately deletable. When you need to change deletion logic, you open deleteUser — not a 60-line function where deletion is one third of a conditional. Each function can also accept only the parameters it actually needs, instead of a blob that suits every mode.
05 — The hidden cost of not extracting utils and small functions
When small pieces of logic live inline everywhere they are used, several things quietly break over time. The logic gets duplicated — first once, then inevitably again. Bugs get fixed in one place but not another. Tests can't be written against logic that's embedded inside a larger function. And readers have to parse implementation details every time they want to understand intent.
"Inline logic optimizes for writing. Named functions optimize for reading, testing, and changing."
Extracting a small function costs almost nothing. Naming it well is free documentation. Moving it to a utils file makes it discoverable. The discipline of extraction also forces clarity: if you can't name a function, you might not fully understand what it does.
pattern
// Scattered inline
const label = user.firstName[0].toUpperCase() +
user.lastName[0].toUpperCase();
// Extracted, named, testable
const getInitials = (user) =>
`${user.firstName[0]}${user.lastName[0]}`.toUpperCase();06 — Boolean parameters as a smell
A boolean parameter tells you that the function is doing two things. At the call site, render(true) is completely unreadable. You have to jump to the definition to understand what true means. Booleans in function signatures are a symptom of a function that wasn't fully split — or a missing concept.
smell
createUser(userData, true);
// What does true mean?
function createUser(data, isAdmin) {
...
}clean
createAdminUser(userData);
createRegularUser(userData);
// or with an options object
createUser(userData, { role: 'admin' });Options objects are the friendlier alternative when you genuinely need a configuration. They're self-documenting at the call site and easy to extend later without breaking signatures.
07 — Nested conditionals vs early returns
Deep nesting is a tax on the reader. Every level of indentation adds to the mental stack the reader must hold while processing the function. Early returns — also called guard clauses — invert the structure: handle the failure cases first and return, so the happy path is flat and unencumbered.
smell
function processOrder(order) {
if (order) {
if (order.isPaid) {
if (order.items.length > 0) {
// actual logic buried here
}
}
}
}clean
function processOrder(order) {
if (!order) return;
if (!order.isPaid) return;
if (!order.items.length) return;
// actual logic is flat here
}The clean version reads like a checklist. Each precondition is explicit and standalone. The core logic sits at the base indentation level, where it belongs.
08 — Magic numbers and strings
A magic number is a literal value in the code with no explanation of what it represents. 86400 means nothing. SECONDS_PER_DAY = 86400 means everything. The same applies to strings: 'admin' scattered through the codebase is a ticking time bomb; a shared constant is a single source of truth.
smell
if (user.role === 'admin') { ... }
setTimeout(refresh, 300000);
const limit = 25;clean
const ROLES = { ADMIN: 'admin' };
const REFRESH_INTERVAL_MS = 300_000;
const PAGE_SIZE = 25;
if (user.role === ROLES.ADMIN) { ... }Constants collected in one place are easy to audit, easy to change, and self-explanatory. The numeric separator (300_000) is especially useful for large numbers where digit grouping aids readability.
09 — Variable naming should remove the need for comments
If you need a comment to explain what a variable holds, the variable name is doing partial work. A good name encodes the type, the intent, and the context in a few words. This is one of the highest-leverage habits in writing readable code.
smell
// list of users who opted in
const data = res.filter(u => u.opted);
// elapsed time in ms
const t = Date.now() - start;clean
const optedInUsers = res.filter(
u => u.opted
);
const elapsedMs = Date.now() - start;The comments in the "smell" example are not wrong — they're redundant. The well-named version makes them unnecessary. Every comment you don't need to write is a comment that can't drift out of sync with the code.
10 — Comments explain "why", not "what"
Code already explains what it does — that's its job. Comments should fill in what code cannot: the reasoning behind a non-obvious decision, a constraint imposed by an external system, a known limitation, or a warning to future developers who might be tempted to "clean up" something that looks wrong but is intentional.
what (redundant)
// Loop through users
users.forEach(user => {
// Send email to user
sendEmail(user.email);
});why (valuable)
// Stripe requires sequential charges;
// parallel calls hit rate limits (2023 Q2)
for (const user of users) {
await chargeUser(user);
}The "why" comment survives refactoring. Even if the code around it changes, the constraint it documents remains relevant. "What" comments age badly — they become lies the moment the code changes and someone forgets to update them.
11 — Avoid primitive obsession
Primitive obsession is the habit of representing domain concepts as plain strings, numbers, or booleans rather than named types or objects. An email address is not just a string. A price is not just a number. When your domain concepts are primitives, nothing prevents you from passing a username where an email is expected, or a price where a quantity belongs.
smell
function sendInvoice(
email: string,
amount: number,
currency: string
) { ... }clean
type Email = { address: string };
type Money = { amount: number; currency: string };
function sendInvoice(
to: Email,
total: Money
) { ... }Named types make invalid states harder to represent. They group related data together. They give your domain a vocabulary that makes the code read closer to how the business thinks.
12 — Don't return different shapes from the same function
A function that sometimes returns null, sometimes an array, and sometimes an object forces every caller to write defensive guards for three possible shapes. This unpredictability is a form of dishonesty — the function's signature doesn't tell the full story.
smell
function getUser(id) {
if (!id) return null;
if (id === 'all') return [];
return { id, name: '...' };
}clean
function getUser(id: string): User | null {
if (!id) return null;
return { id, name: '...' };
}
function getAllUsers(): User[] { ... }Consistent return shapes allow callers to be simple. TypeScript makes this contractual. Even without types, the discipline of returning one shape builds functions that are easier to compose, test, and trust.
13 — Keep side effects separate from pure logic
A function that both computes something and modifies external state is doing two jobs. The computation part — the pure logic — is easy to test in isolation. The side effect part — writing to a database, making an API call, updating global state — is not. When they're combined, you can't test one without triggering the other.
smell
function applyDiscount(cart) {
const total = cart.items
.reduce((s, i) => s + i.price, 0);
const discounted = total * 0.9;
analytics.track('discount_applied');
db.save({ cartId: cart.id, discounted });
return discounted;
}clean
// Pure — easily testable
function calculateDiscount(cart) {
const total = cart.items
.reduce((s, i) => s + i.price, 0);
return total * 0.9;
}
// Side effects contained
async function applyDiscount(cart) {
const discounted = calculateDiscount(cart);
await db.save({ cartId: cart.id, discounted });
analytics.track('discount_applied');
return discounted;
}The pure function can be unit tested with a plain object and a strict equality check. The side-effect function can be integration tested separately. This separation also makes the code easier to reason about — you always know which functions change the world and which ones only observe it.
These aren't rules to follow blindly. They're patterns that emerge from a single underlying principle: code is read far more often than it is written. Every decision in this list is a decision to make the next reader's life easier — even when that reader is you, six months from now.
clean code react patterns refactoring code smells frontend craft software design