[G1-Sync] Manual knowledge update
This commit is contained in:
@@ -0,0 +1,403 @@
|
||||
---
|
||||
id: quality-code-smells
|
||||
title: Code Smells — 변경 어려움의 signal
|
||||
category: Coding
|
||||
status: draft
|
||||
source_trust_level: B
|
||||
verification_status: conceptual
|
||||
created_at: 2026-05-09
|
||||
updated_at: 2026-05-09
|
||||
tags: [quality, refactoring, vibe-coding]
|
||||
tech_stack: { language: "TS / generic", applicable_to: ["Engineering"] }
|
||||
applied_in: []
|
||||
aliases: [code smell, long method, god object, primitive obsession, feature envy, shotgun surgery]
|
||||
---
|
||||
|
||||
# Code Smells
|
||||
|
||||
> 코드 가 "이상하다" — bug 아닌 변경 어려움 / 이해 어려움 의 signal. **Long method, god class, primitive obsession, feature envy, shotgun surgery, dead code**.
|
||||
|
||||
## 📖 핵심 개념
|
||||
- Smell ≠ bug — 동작 OK, 변경 비용 ↑.
|
||||
- 매 smell 가 reasonable 일 때 있음 — 문맥.
|
||||
- Refactoring 가 답 — 작은 step.
|
||||
- Linter 가 일부 자동 감지.
|
||||
|
||||
## 💻 코드 패턴
|
||||
|
||||
### Long method
|
||||
```ts
|
||||
// ❌ 100 줄 함수
|
||||
function processOrder(order) {
|
||||
// validate
|
||||
if (!order.userId) throw ...;
|
||||
if (!order.items) throw ...;
|
||||
// ...
|
||||
|
||||
// calculate total
|
||||
let total = 0;
|
||||
for (const item of order.items) {
|
||||
total += item.price * item.qty;
|
||||
}
|
||||
// tax, discount, ...
|
||||
|
||||
// save
|
||||
// ...
|
||||
|
||||
// notify
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ Extract method
|
||||
function processOrder(order) {
|
||||
validateOrder(order);
|
||||
const total = calculateTotal(order);
|
||||
saveOrder(order, total);
|
||||
notifyUser(order);
|
||||
}
|
||||
```
|
||||
|
||||
→ 한 함수 1 책임 — 30 줄 이하 권장.
|
||||
|
||||
### God class
|
||||
```ts
|
||||
// ❌ User 가 모든 거
|
||||
class User {
|
||||
login() { ... }
|
||||
logout() { ... }
|
||||
sendEmail() { ... } // 메일 책임?
|
||||
generateInvoice() { ... } // 인보이스?
|
||||
trackAnalytics() { ... } // 분석?
|
||||
}
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ 분리
|
||||
class User { login(); logout(); }
|
||||
class Mailer { sendEmail(user); }
|
||||
class InvoiceService { generate(user); }
|
||||
```
|
||||
|
||||
→ Single Responsibility. 매 class 1 reason to change.
|
||||
|
||||
### Primitive obsession
|
||||
```ts
|
||||
// ❌ string 만
|
||||
function transfer(fromId: string, toId: string, amount: number) { ... }
|
||||
// → fromId 가 user / account / order? amount 가 cents / dollars?
|
||||
|
||||
// ✅ Branded / value object
|
||||
type UserId = string & { __brand: 'UserId' };
|
||||
type AccountId = string & { __brand: 'AccountId' };
|
||||
|
||||
class Money {
|
||||
constructor(public cents: number, public currency: string) {}
|
||||
add(other: Money): Money { ... }
|
||||
}
|
||||
|
||||
function transfer(from: AccountId, to: AccountId, amount: Money) { ... }
|
||||
```
|
||||
|
||||
### Feature envy
|
||||
```ts
|
||||
// ❌ Order 가 user 의 data 만 사용
|
||||
class OrderService {
|
||||
total(order: Order, user: User) {
|
||||
let t = order.total;
|
||||
if (user.isVip) t *= 0.9;
|
||||
if (user.country === 'US') t += 0.08 * t;
|
||||
return t;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ User method
|
||||
class User {
|
||||
applyDiscount(amount: number): number {
|
||||
let r = amount;
|
||||
if (this.isVip) r *= 0.9;
|
||||
if (this.country === 'US') r += 0.08 * r;
|
||||
return r;
|
||||
}
|
||||
}
|
||||
|
||||
class OrderService {
|
||||
total(order: Order, user: User) {
|
||||
return user.applyDiscount(order.total);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Shotgun surgery
|
||||
```
|
||||
1 변경 = N 파일 수정.
|
||||
|
||||
예: "User 의 status 가 'banned' 추가"
|
||||
- User type
|
||||
- 모든 query (deleted=false → status='active')
|
||||
- 모든 UI badge
|
||||
- ...
|
||||
|
||||
→ Single responsibility 위반. Encapsulate.
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ Status 가 1 곳에
|
||||
class UserStatus {
|
||||
static active(): UserStatus { ... }
|
||||
static banned(): UserStatus { ... }
|
||||
isVisible(): boolean { ... }
|
||||
badge(): { color: string; text: string } { ... }
|
||||
}
|
||||
```
|
||||
|
||||
### Long parameter list
|
||||
```ts
|
||||
// ❌
|
||||
function createUser(
|
||||
name: string, email: string, password: string,
|
||||
birthdate: Date, country: string, city: string,
|
||||
newsletter: boolean, ...
|
||||
) { ... }
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ Parameter object
|
||||
interface CreateUserInput {
|
||||
name: string;
|
||||
email: string;
|
||||
// ...
|
||||
}
|
||||
|
||||
function createUser(input: CreateUserInput) { ... }
|
||||
```
|
||||
|
||||
### Data clump
|
||||
```ts
|
||||
// ❌ 같은 3 param 자주
|
||||
function shipping(street, city, zip) { ... }
|
||||
function billing(street, city, zip) { ... }
|
||||
|
||||
// ✅ Address class
|
||||
class Address { ... }
|
||||
function shipping(addr: Address) { ... }
|
||||
```
|
||||
|
||||
### Comments (smell)
|
||||
```ts
|
||||
// ❌ 코드 가 이해 안 됨 → comment 로 보충
|
||||
// Calculate total with tax and discount
|
||||
function fn(o, u) {
|
||||
let t = o.t;
|
||||
// VIP discount
|
||||
if (u.v) t *= 0.9;
|
||||
// ...
|
||||
}
|
||||
|
||||
// ✅ 변수 / 함수 이름
|
||||
function calculateTotalWithTaxAndDiscount(order, user) {
|
||||
let total = order.subtotal;
|
||||
total = applyVipDiscount(total, user);
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
→ Comment = 코드 가 부족 의 signal. Naming 으로 fix.
|
||||
|
||||
### Dead code
|
||||
```ts
|
||||
// ❌ 안 쓰이는 함수
|
||||
function oldMethod() { ... } // 아무도 호출 X
|
||||
|
||||
// ✅ 삭제
|
||||
// Git history 가 보존
|
||||
```
|
||||
|
||||
→ "혹시" 안 둠. Lint 가 감지 (no-unused-vars).
|
||||
|
||||
### Duplicate code
|
||||
```ts
|
||||
// ❌ 같은 logic 두 곳
|
||||
function validateUserA(user) {
|
||||
if (!user.email) throw ...;
|
||||
if (!user.email.includes('@')) throw ...;
|
||||
}
|
||||
function validateUserB(user) {
|
||||
if (!user.email) throw ...;
|
||||
if (!user.email.includes('@')) throw ...;
|
||||
}
|
||||
|
||||
// ✅ Extract
|
||||
function validateEmail(email: string) {
|
||||
if (!email) throw ...;
|
||||
if (!email.includes('@')) throw ...;
|
||||
}
|
||||
```
|
||||
|
||||
→ Rule of three: 3 번 = abstract. 2 번 = OK.
|
||||
|
||||
### Switch / if cascade
|
||||
```ts
|
||||
// ❌ Type 별 분기 폭발
|
||||
switch (animal.type) {
|
||||
case 'dog': return bark(animal);
|
||||
case 'cat': return meow(animal);
|
||||
case 'cow': return moo(animal);
|
||||
// 새 type → 모든 switch 수정 (shotgun surgery)
|
||||
}
|
||||
```
|
||||
|
||||
```ts
|
||||
// ✅ Polymorphism
|
||||
interface Animal { sound(): string; }
|
||||
class Dog implements Animal { sound() { return 'bark'; } }
|
||||
class Cat implements Animal { sound() { return 'meow'; } }
|
||||
|
||||
animal.sound();
|
||||
```
|
||||
|
||||
### Magic number / string
|
||||
```ts
|
||||
// ❌
|
||||
if (user.role === 3) ...
|
||||
setTimeout(fn, 86400000);
|
||||
|
||||
// ✅
|
||||
const ROLE_ADMIN = 3;
|
||||
const ONE_DAY_MS = 24 * 60 * 60 * 1000;
|
||||
```
|
||||
|
||||
### Lazy class (작은 class)
|
||||
```ts
|
||||
// ❌ 의미 없는 wrapper
|
||||
class UserName {
|
||||
constructor(public name: string) {}
|
||||
}
|
||||
|
||||
// ✅ string 직접 — 쓰기 nothing 추가 시.
|
||||
// 단, validation / brand 가 있으면 유지.
|
||||
```
|
||||
|
||||
### Speculative generality
|
||||
```ts
|
||||
// ❌ "혹시 나중" 추상
|
||||
abstract class BaseHandler { abstract process(input: any): any; }
|
||||
class SingleConcreteHandler extends BaseHandler { ... }
|
||||
// → 1 구현 만 — abstract 의미 없음
|
||||
|
||||
// ✅ YAGNI. Concrete first, abstract when 2nd 구현.
|
||||
```
|
||||
|
||||
### Refused bequest
|
||||
```ts
|
||||
// ❌ 상속 거부
|
||||
class Bird { fly() {} }
|
||||
class Penguin extends Bird {
|
||||
fly() { throw new Error('penguins cant fly'); } // 상속 거부
|
||||
}
|
||||
|
||||
// ✅ 다른 추상
|
||||
interface Animal { ... }
|
||||
class Bird implements Animal { fly() {} }
|
||||
class Penguin implements Animal { swim() {} }
|
||||
```
|
||||
|
||||
### Inappropriate intimacy
|
||||
```ts
|
||||
// ❌ 다른 class 의 internal 접근
|
||||
class Order {
|
||||
total(user: User) {
|
||||
return this.amount * user._private.discount; // private 접근
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ Public method
|
||||
class User { discount(): number { return this._private.discount; } }
|
||||
```
|
||||
|
||||
### Train wreck / Law of Demeter
|
||||
```ts
|
||||
// ❌ 깊은 chain
|
||||
const city = order.user.address.city.name;
|
||||
|
||||
// ✅ Tell, don't ask
|
||||
const city = order.userCity();
|
||||
```
|
||||
|
||||
→ "한 dot rule". 내부 구조 노출 X.
|
||||
|
||||
### Temporal coupling
|
||||
```ts
|
||||
// ❌ 순서 dependency
|
||||
const x = new X();
|
||||
x.init(); // 잊으면 깨짐
|
||||
x.process();
|
||||
|
||||
// ✅ Constructor 가 init
|
||||
const x = new X(); // 자동 init
|
||||
x.process();
|
||||
```
|
||||
|
||||
### Linter / 자동 감지
|
||||
```bash
|
||||
# eslint-plugin-sonarjs (smell)
|
||||
yarn add -D eslint-plugin-sonarjs
|
||||
|
||||
# rules: cognitive-complexity, no-duplicate-string, no-identical-functions, etc.
|
||||
```
|
||||
|
||||
### Sonar / CodeClimate
|
||||
```
|
||||
- Cyclomatic complexity > 10
|
||||
- Cognitive complexity > 15
|
||||
- Duplication > 5%
|
||||
- File LOC > 500
|
||||
- Method LOC > 30
|
||||
|
||||
→ 자동 보고.
|
||||
```
|
||||
|
||||
### Refactor 시점
|
||||
```
|
||||
"Smell 발견 → 즉시 fix" 가 ideal.
|
||||
실제: feature 작업 중 발견 → 별 PR (boy scout rule).
|
||||
|
||||
큰 refactor = 따로 sprint.
|
||||
```
|
||||
|
||||
## 🤔 의사결정 기준
|
||||
| Smell | Refactor |
|
||||
|---|---|
|
||||
| Long method | Extract method |
|
||||
| God class | Extract class / SRP |
|
||||
| Primitive obsession | Value object / branded |
|
||||
| Feature envy | Move method |
|
||||
| Shotgun surgery | Encapsulate |
|
||||
| Long params | Parameter object |
|
||||
| Data clump | Class |
|
||||
| Duplicate | Extract method (3+) |
|
||||
| Switch on type | Polymorphism |
|
||||
| Magic number | Constant |
|
||||
| Train wreck | Tell don't ask |
|
||||
|
||||
## ❌ 안티패턴
|
||||
- **모든 smell 즉시 fix**: 큰 refactor 가 PR 막힘.
|
||||
- **무시**: 누적 = 변경 어려움.
|
||||
- **Smell 가 패턴 으로 위장**: e.g. "이건 strategy 인데" — 1 구현.
|
||||
- **Comment 가 변수명 대체**: code 가 분명해야.
|
||||
- **YAGNI 무시 (speculative)**: 안 쓰이는 추상.
|
||||
- **DRY 강박 (premature abstract)**: 2번 = OK, 3번 = extract.
|
||||
|
||||
## 🤖 LLM 활용 힌트
|
||||
- LLM 가 smell 찾기 강함 (PR review).
|
||||
- "Boy scout rule" — 작은 fix 매번.
|
||||
- Linter (sonarjs) 자동 감지.
|
||||
- Refactor = 작은 commit + test 보존.
|
||||
|
||||
## 🔗 관련 문서
|
||||
- [[Quality_Refactoring]]
|
||||
- [[Productivity_Code_Review]]
|
||||
- [[Quality_Tech_Debt]]
|
||||
Reference in New Issue
Block a user