clean-code

クリーンコードスキル

クリーンコードの原則に従って、読みやすく保守しやすいコードを書きます。

使用する場面

• ユーザーが「このコードをきれいにして」「リファクタリングして」「読みやすくして」と依頼したとき
• 保守性に重点を置いたコードレビュー
• 複雑さの軽減
• 命名の改善

---

基本原則

原則	意味	違反の兆候
DRY	同じことを繰り返さない	コードブロックのコピー＆ペースト
KISS	できるだけシンプルにする	過剰に設計されたソリューション
YAGNI	必要になるまで作らない	「念のため」の機能

---

DRY - 同じことを繰り返さない

「知識のすべての要素は、システム内で単一かつ曖昧さのない形で表現されなければなりません。」

違反例

// ❌ BAD: Same validation logic repeated
public class UserController {

    public void createUser(UserRequest request) {
        if (request.getEmail() == null || request.getEmail().isBlank()) {
            throw new ValidationException("Email is required");
        }
        if (!request.getEmail().contains("@")) {
            throw new ValidationException("Invalid email format");
        }
        // ... create user
    }

    public void updateUser(UserRequest request) {
        if (request.getEmail() == null || request.getEmail().isBlank()) {
            throw new ValidationException("Email is required");
        }
        if (!request.getEmail().contains("@")) {
            throw new ValidationException("Invalid email format");
        }
        // ... update user
    }
}

リファクタリング後

// ✅ GOOD: Single source of truth
public class EmailValidator {

    public void validate(String email) {
        if (email == null || email.isBlank()) {
            throw new ValidationException("Email is required");
        }
        if (!email.contains("@")) {
            throw new ValidationException("Invalid email format");
        }
    }
}

public class UserController {
    private final EmailValidator emailValidator;

    public void createUser(UserRequest request) {
        emailValidator.validate(request.getEmail());
        // ... create user
    }

    public void updateUser(UserRequest request) {
        emailValidator.validate(request.getEmail());
        // ... update user
    }
}

DRYの例外

重複がすべて悪いわけではありません。時期尚早な抽象化は避けましょう。

// These look similar but serve different purposes - OK to duplicate
public BigDecimal calculateShippingCost(Order order) {
    return order.getWeight().multiply(SHIPPING_RATE);
}

public BigDecimal calculateInsuranceCost(Order order) {
    return order.getValue().multiply(INSURANCE_RATE);
}
// Don't force these into one method - they'll evolve differently

---

KISS - シンプルに保つ

「通常、最もシンプルなソリューションが最善です。」

違反例

// ❌ BAD: Over-engineered for simple task
public class StringUtils {

    public boolean isEmpty(String str) {
        return Optional.ofNullable(str)
            .map(String::trim)
            .map(String::isEmpty)
            .orElseGet(() -> Boolean.TRUE);
    }
}

リファクタリング後

// ✅ GOOD: Simple and clear
public class StringUtils {

    public boolean isEmpty(String str) {
        return str == null || str.trim().isEmpty();
    }

    // Or use existing library
    // return StringUtils.isBlank(str);  // Apache Commons
    // return str == null || str.isBlank();  // Java 11+
}

KISSチェックリスト

• ジュニア開発者がこれを30秒で理解できますか？
• 標準ライブラリを使う、よりシンプルな方法はありますか？
• 発生しないかもしれないエッジケースのために複雑さを加えていませんか？

---

YAGNI - 必要になるまで作らない

「必要になるまで機能を追加しないでください。」

違反例

// ❌ BAD: Building for hypothetical future
public interface Repository<T, ID> {
    T findById(ID id);
    List<T> findAll();
    List<T> findAll(Pageable pageable);
    List<T> findAll(Sort sort);
    List<T> findAllById(Iterable<ID> ids);
    T save(T entity);
    List<T> saveAll(Iterable<T> entities);
    void delete(T entity);
    void deleteById(ID id);
    void deleteAll(Iterable<T> entities);
    void deleteAll();
    boolean existsById(ID id);
    long count();
    // ... 20 more methods "just in case"
}

// Current usage: only findById and save

リファクタリング後

// ✅ GOOD: Only what's needed now
public interface UserRepository {
    Optional<User> findById(Long id);
    User save(User user);
}

// Add methods when actually needed, not before

YAGNIの兆候

• 「後で必要になるかもしれません」
• 「念のため設定可能にしておきましょう」
• 「将来Xをサポートする必要が出たらどうしますか？」
• 実装が1つしかない抽象クラス

---

命名規則

変数

// ❌ BAD
int d;                  // What is d?
String s;               // Meaningless
List<User> list;        // What kind of list?
Map<String, Object> m;  // What does it map?

// ✅ GOOD
int elapsedTimeInDays;
String customerName;
List<User> activeUsers;
Map<String, Object> sessionAttributes;

ブール値

// ❌ BAD
boolean flag;
boolean status;
boolean check;

// ✅ GOOD - Use is/has/can/should prefix
boolean isActive;
boolean hasPermission;
boolean canEdit;
boolean shouldNotify;

メソッド

// ❌ BAD
void process();           // Process what?
void handle();            // Handle what?
void doIt();              // Do what?
User get();               // Get from where?

// ✅ GOOD - Verb + noun, descriptive
void processPayment();
void handleLoginRequest();
void sendWelcomeEmail();
User findByEmail(String email);
List<Order> fetchPendingOrders();

クラス

// ❌ BAD
class Data { }           // Too vague
class Info { }           // Too vague
class Manager { }        // Often a god class
class Helper { }         // Often a dumping ground
class Utils { }          // Static method dumping ground

// ✅ GOOD - Noun, specific responsibility
class User { }
class OrderProcessor { }
class EmailValidator { }
class PaymentGateway { }
class ShippingCalculator { }

命名規則の表

要素	規則	例
クラス	パスカルケース、名詞	OrderService
インターフェース	パスカルケース、形容詞または名詞	Comparable、List
メソッド	キャメルケース、動詞	calculateTotal()
変数	キャメルケース、名詞	customerEmail
定数	大文字のスネークケース	MAX_RETRY_COUNT
パッケージ	小文字	com.example.orders

---

関数／メソッド

関数を小さく保つ

// ❌ BAD: 50+ line method doing multiple things
public void processOrder(Order order) {
    // validate order (10 lines)
    // calculate totals (15 lines)
    // apply discounts (10 lines)
    // update inventory (10 lines)
    // send notifications (10 lines)
    // ... and more
}

// ✅ GOOD: Small, focused methods
public void processOrder(Order order) {
    validateOrder(order);
    calculateTotals(order);
    applyDiscounts(order);
    updateInventory(order);
    sendNotifications(order);
}

抽象度をそろえる

// ❌ BAD: Mixed abstraction levels
public void processOrder(Order order) {
    validateOrder(order);  // High level

    // Low level mixed in
    BigDecimal total = BigDecimal.ZERO;
    for (OrderItem item : order.getItems()) {
        total = total.add(item.getPrice().multiply(
            BigDecimal.valueOf(item.getQuantity())));
    }

    sendEmail(order);  // High level again
}

// ✅ GOOD: Consistent abstraction level
public void processOrder(Order order) {
    validateOrder(order);
    calculateTotal(order);
    sendConfirmation(order);
}

private BigDecimal calculateTotal(Order order) {
    return order.getItems().stream()
        .map(item -> item.getPrice().multiply(
            BigDecimal.valueOf(item.getQuantity())))
        .reduce(BigDecimal.ZERO, BigDecimal::add);
}

パラメーターを制限する

// ❌ BAD: Too many parameters
public User createUser(String firstName, String lastName,
                       String email, String phone,
                       String address, String city,
                       String country, String zipCode) {
    // ...
}

// ✅ GOOD: Use parameter object
public User createUser(CreateUserRequest request) {
    // ...
}

// Or builder
public User createUser(UserBuilder builder) {
    // ...
}

フラグ引数を避ける

// ❌ BAD: Boolean flag changes behavior
public void sendMessage(String message, boolean isUrgent) {
    if (isUrgent) {
        // send immediately
    } else {
        // queue for later
    }
}

// ✅ GOOD: Separate methods
public void sendUrgentMessage(String message) {
    // send immediately
}

public void queueMessage(String message) {
    // queue for later
}

---

コメント

明白なコメントを避ける

// ❌ BAD: Noise comments
// Set the user's name
user.setName(name);

// Increment counter
counter++;

// Check if user is null
if (user != null) {
    // ...
}

良いコメント

// ✅ GOOD: Explain WHY, not WHAT

// Retry with exponential backoff to avoid overwhelming the server
// during high load periods (see incident #1234)
for (int attempt = 0; attempt < MAX_RETRIES; attempt++) {
    Thread.sleep((long) Math.pow(2, attempt) * 1000);
    // ...
}

// TODO: Replace with Redis cache after infrastructure upgrade (Q2 2026)
private Map<String, User> userCache = new ConcurrentHashMap<>();

// WARNING: Order matters! Discounts must be applied before tax calculation
applyDiscounts(order);
calculateTax(order);

コードに語らせる

// ❌ BAD: Comment explaining bad code
// Check if the user is an admin or has special permission
// and the action is allowed for their role
if ((user.getRole() == 1 || user.getRole() == 2) &&
    (action == 3 || action == 4 || action == 7)) {
    // ...
}

// ✅ GOOD: Self-documenting code
if (user.hasAdminPrivileges() && action.isAllowedFor(user.getRole())) {
    // ...
}

---

よくあるコードの不吉な臭い

臭い	説明	リファクタリング
長いメソッド	メソッドが20行を超える	メソッドの抽出
長い引数リスト	引数が3つを超える	パラメーターオブジェクト
重複コード	複数箇所に同じコードがある	メソッド／クラスの抽出
デッドコード	使用されていないコード	削除する
マジックナンバー	説明のないリテラル	名前付き定数
ゴッドクラス	クラスが多くの処理を担っている	クラスの抽出
フィーチャーエンヴィ	メソッドが別のクラスのデータを使用している	メソッドの移動
プリミティブ執着	オブジェクトの代わりにプリミティブを使っている	値オブジェクト

マジックナンバー

// ❌ BAD
if (user.getAge() >= 18) { }
if (order.getTotal() > 100) { }
Thread.sleep(86400000);

// ✅ GOOD
private static final int ADULT_AGE = 18;
private static final BigDecimal FREE_SHIPPING_THRESHOLD = new BigDecimal("100");
private static final long ONE_DAY_MS = TimeUnit.DAYS.toMillis(1);

if (user.getAge() >= ADULT_AGE) { }
if (order.getTotal().compareTo(FREE_SHIPPING_THRESHOLD) > 0) { }
Thread.sleep(ONE_DAY_MS);

プリミティブ執着

// ❌ BAD: Primitives everywhere
public void createUser(String email, String phone, String zipCode) {
    // No validation, easy to mix up parameters
}

createUser("12345", "[email protected]", "555-1234");  // Wrong order, compiles!

// ✅ GOOD: Value objects
public record Email(String value) {
    public Email {
        if (!value.contains("@")) {
            throw new IllegalArgumentException("Invalid email");
        }
    }
}

public record PhoneNumber(String value) {
    // validation
}

public void createUser(Email email, PhoneNumber phone, ZipCode zipCode) {
    // Type-safe, self-validating
}

---

リファクタリングのクイックリファレンス

変更前	変更後	手法
長いメソッド	短いメソッド	メソッドの抽出
重複コード	単一のメソッド	メソッドの抽出
複雑な条件分岐	ポリモーフィズム	条件分岐をポリモーフィズムに置き換える
多すぎるパラメーター	オブジェクト	パラメーターオブジェクトの導入
一時変数	クエリメソッド	一時変数をクエリに置き換える
コードを説明するコメント	自己文書化コード	名前変更、抽出
ネストされた条件分岐	早期リターン	ガード節

ガード節

// ❌ BAD: Deeply nested
public void processOrder(Order order) {
    if (order != null) {
        if (order.isValid()) {
            if (order.hasItems()) {
                // actual logic buried here
            }
        }
    }
}

// ✅ GOOD: Guard clauses
public void processOrder(Order order) {
    if (order == null) return;
    if (!order.isValid()) return;
    if (!order.hasItems()) return;

    // actual logic at top level
}

---

クリーンコードチェックリスト

コードをレビューする際に確認します。

• 名前は意味が明確で、発音しやすいですか？
• 関数は小さく、目的が絞られていますか？
• 重複したコードはありませんか？
• マジックナンバーやマジック文字列はありませんか？
• コメントは「何を」ではなく「なぜ」を説明していますか？
• コードの抽象度は一貫していますか？
• 簡潔にできるコードはありませんか？
• デッドコードや未使用コードはありませんか？

---

関連スキル

• solid-principles - クラス構造の設計原則
• design-patterns - 繰り返し発生する問題に対する一般的な解決策
• java-code-review - 包括的なレビュー用チェックリスト