We've all been there. You open a pull request from a teammate, start reading through the code, and within 30 seconds you can tell how experienced the person is.
Not because the code doesn't work. It probably does. But there are patterns -- "code smells" -- that experienced devs spot instantly. They're like a neon sign saying "I'm still learning."
I'm Daniil, 19, and I've been writing code since I was 14. I've made every single mistake on this list. Multiple times. Here's what I learned so I could stop making them.
1. God Objects That Do Everything
This is probably the most common one. You have a single class or module that handles authentication, database queries, email sending, logging, and probably makes coffee too.
The smell:
class UserManager:
def authenticate(self, username, password):
# 50 lines of auth logic
pass
def send_welcome_email(self, user):
# 30 lines of email logic
pass
def generate_report(self, user):
# 40 lines of report generation
pass
def update_payment(self, user, card):
# 60 lines of payment processing
pass
def resize_avatar(self, user, image):
# 25 lines of image processing
pass
The fix:
class AuthService:
def authenticate(self, username, password):
pass
class EmailService:
def send_welcome_email(self, user):
pass
class ReportGenerator:
def generate(self, user):
pass
class PaymentProcessor:
def update_payment(self, user, card):
pass
Each class has one job. If you can't describe what your class does without using the word "and," it's doing too much.
2. Magic Numbers Everywhere
When you see raw numbers scattered through code with zero explanation, it's a red flag. What does 86400 mean? What about 3? Or 0.15?
The smell:
if (retryCount > 3) {
throw new Error("Failed");
}
const timeout = 86400;
const price = basePrice * 1.15;
if (status === 2) {
// do something
}
The fix:
const MAX_RETRIES = 3;
const ONE_DAY_IN_SECONDS = 86400;
const TAX_RATE = 0.15;
const STATUS = {
PENDING: 0,
ACTIVE: 1,
SUSPENDED: 2
};
if (retryCount > MAX_RETRIES) {
throw new Error("Failed");
}
const timeout = ONE_DAY_IN_SECONDS;
const price = basePrice * (1 + TAX_RATE);
if (status === STATUS.SUSPENDED) {
// now we know what this means
}
Future you (and everyone on your team) will thank you.
3. Force Unwrapping and No Error Handling
This one hits especially hard in Swift, but it applies everywhere. Just assuming things will work and not handling the case when they don't.
The smell (Swift):
let data = try! JSONDecoder().decode(User.self, from: responseData)
let url = URL(string: urlString)!
let image = UIImage(named: "profile")!
The smell (JavaScript):
async function getUser(id) {
const response = await fetch(`/api/users/${id}`);
const data = await response.json();
return data.user.name.first;
}
No error handling. No null checks. If anything goes wrong, the app crashes and you have no idea why.
The fix (Swift):
guard let url = URL(string: urlString) else {
logger.error("Invalid URL: \(urlString)")
return
}
do {
let data = try JSONDecoder().decode(User.self, from: responseData)
} catch {
logger.error("Decoding failed: \(error)")
}
The fix (JavaScript):
async function getUser(id) {
try {
const response = await fetch(`/api/users/${id}`);
if (!response.ok) {
throw new Error(`HTTP ${response.status}`);
}
const data = await response.json();
return data?.user?.name?.first ?? "Unknown";
} catch (error) {
console.error(`Failed to fetch user ${id}:`, error);
return null;
}
}
Always ask yourself: "What happens if this fails?"
4. Comments That Explain the Obvious
Beginners often think more comments = better code. But comments that just repeat what the code already says are noise.
The smell:
# increment counter by 1
counter += 1
# check if user is admin
if user.is_admin:
# grant admin access
grant_admin_access(user)
# loop through all users
for user in users:
# send email to user
send_email(user)
The fix:
counter += 1
if user.is_admin:
grant_admin_access(user)
# Skip users who opted out of notifications in Q4 rollout
# See ticket PROJ-1234 for context
for user in active_notification_users:
send_email(user)
Good comments explain why, not what. The code itself should explain what it does. If it doesn't, rename your variables and functions.
5. Copy-Paste Programming
You need similar functionality in two places, so you copy the code block and change a few things. Then you need it in a third place. Copy again. Now you have a bug in the original, and you need to fix it in 3 places. Except you forget the third one.
The smell:
// In UserController
const user = await db.query("SELECT * FROM users WHERE id = ?", [id]);
if (!user) {
return res.status(404).json({ error: "Not found" });
}
const sanitized = { id: user.id, name: user.name, email: user.email };
// In AdminController (same thing, slightly different)
const user = await db.query("SELECT * FROM users WHERE id = ?", [id]);
if (!user) {
return res.status(404).json({ error: "Not found" });
}
const sanitized = { id: user.id, name: user.name, email: user.email, role: user.role };
// In ProfileController (again!)
const user = await db.query("SELECT * FROM users WHERE id = ?", [id]);
if (!user) {
return res.status(404).json({ error: "Not found" });
}
The fix:
class UserRepository {
async findById(id) {
const user = await db.query("SELECT * FROM users WHERE id = ?", [id]);
if (!user) throw new NotFoundError("User not found");
return user;
}
sanitize(user, includeRole = false) {
const base = { id: user.id, name: user.name, email: user.email };
if (includeRole) base.role = user.role;
return base;
}
}
The DRY principle exists for a reason. If you find yourself copying code, stop and extract it.
6. Deeply Nested Conditionals
When your code starts looking like a sideways pyramid, something has gone wrong.
The smell:
def process_order(order):
if order:
if order.is_valid:
if order.has_items:
if order.user.is_active:
if order.payment_method:
if check_inventory(order):
process_payment(order)
return "Success"
else:
return "Out of stock"
else:
return "No payment method"
else:
return "User inactive"
else:
return "No items"
else:
return "Invalid order"
else:
return "No order"
The fix -- use early returns (guard clauses):
def process_order(order):
if not order:
return "No order"
if not order.is_valid:
return "Invalid order"
if not order.has_items:
return "No items"
if not order.user.is_active:
return "User inactive"
if not order.payment_method:
return "No payment method"
if not check_inventory(order):
return "Out of stock"
process_payment(order)
return "Success"
Same logic. Way easier to read. Guard clauses let you handle edge cases first and keep the happy path clean.
7. Not Using Version Control Properly
This isn't a code pattern, but it's a dead giveaway. Things like:
- Committing everything in one giant commit: "fixed stuff"
- Never using branches
- Commenting out code instead of deleting it (that's what git history is for)
- Adding
node_modulesor.envfiles to the repo
The smell:
commit abc123: "updates"
commit def456: "more updates"
commit ghi789: "final version"
commit jkl012: "final final version"
commit mno345: "ok actually final"
The fix:
commit abc123: "feat: add user registration with email verification"
commit def456: "fix: prevent duplicate email signups (closes #42)"
commit ghi789: "refactor: extract payment logic into PaymentService"
commit jkl012: "docs: add API endpoint documentation for /users"
Write commit messages like you're leaving a note for someone who will read this in 6 months with zero context. Because that someone is usually future you.
The Honest Truth
Every experienced developer started by writing code with these exact smells. The difference between a junior and senior developer isn't that seniors never write bad code. It's that they recognize it faster and know how to fix it.
The fact that you're reading articles like this means you're already ahead of the curve. Keep building stuff, keep getting your code reviewed, and keep learning.
What's the worst code smell you've seen in a codebase? Drop it in the comments. I need some entertainment.
If you found this useful, I share more stuff like this on Telegram and sell developer toolkits on Boosty.
Top comments (0)