clean-code-dotnet

Clean Code .NET

Robert C. Martin の Clean Code 原則を、C#/.NET 向けに適用したものです。コードレビューやリファクタリングの際のチェックリストとしてお使いください。

命名

意味のある名前を使う

// ❌ Bad
int d;
var dataFromDb = db.GetFromService().ToList();

// ✅ Good
int daySinceModification;
var employees = _employeeService.GetEmployees().ToList();

ハンガリアン記法を避ける

// ❌ Bad
int iCounter;
string strFullName;
public bool IsShopOpen(string pDay, int pAmount) { }

// ✅ Good
int counter;
string fullName;
public bool IsShopOpen(string day, int amount) { }

発音しやすい名前を使う

// ❌ Bad
public class Employee
{
    public DateTime sWorkDate { get; set; }
    public DateTime modTime { get; set; }
}

// ✅ Good
public class Employee
{
    public DateTime StartWorkingDate { get; set; }
    public DateTime ModificationTime { get; set; }
}

ドメイン名を使う

// ✅ Good - Use patterns developers know
var singletonObject = SingleObject.GetInstance();
var factory = new PatientFactory();
var repository = new PatientRepository();

---

変数

早期リターンを使い、深いネストを避ける

// ❌ Bad - Deep nesting
public bool IsShopOpen(string day)
{
    if (!string.IsNullOrEmpty(day))
    {
        day = day.ToLower();
        if (day == "friday")
        {
            return true;
        }
        else if (day == "saturday")
        {
            return true;
        }
        // ... more nesting
    }
    return false;
}

// ✅ Good - Guard clauses + early return
public bool IsShopOpen(string day)
{
    if (string.IsNullOrEmpty(day))
        return false;

    var openingDays = new[] { "friday", "saturday", "sunday" };
    return openingDays.Contains(day.ToLower());
}

マジックストリングを避ける

// ❌ Bad
if (userRole == "Admin") { }

// ✅ Good
const string AdminRole = "Admin";
if (userRole == AdminRole) { }

// ✅ Better - Use enum
public enum UserRole { Admin, User, Guest }
if (userRole == UserRole.Admin) { }

不要なコンテキストを追加しない

// ❌ Bad - Redundant prefix
public class Car
{
    public string CarMake { get; set; }
    public string CarModel { get; set; }
    public string CarColor { get; set; }
}

// ✅ Good
public class Car
{
    public string Make { get; set; }
    public string Model { get; set; }
    public string Color { get; set; }
}

デフォルト引数を使う

// ❌ Bad
public void CreateMicrobrewery(string name = null)
{
    var breweryName = !string.IsNullOrEmpty(name) ? name : "Hipster Brew Co.";
}

// ✅ Good
public void CreateMicrobrewery(string breweryName = "Hipster Brew Co.")
{
    // breweryName is always valid
}

---

関数

関数は1つのことだけを行う

// ❌ Bad - Multiple responsibilities
public void SendEmailToListOfClients(string[] clients)
{
    foreach (var client in clients)
    {
        var clientRecord = db.Find(client);
        if (clientRecord.IsActive())
        {
            Email(client);
        }
    }
}

// ✅ Good - Single responsibility
public void SendEmailToActiveClients(string[] clients)
{
    var activeClients = GetActiveClients(clients);
    activeClients.ForEach(client => Email(client));
}

public List<Client> GetActiveClients(string[] clients)
{
    return db.Find(clients).Where(c => c.IsActive).ToList();
}

副作用を避ける

// ❌ Bad - Modifies global state
var name = "Ryan McDermott";

public void SplitAndEnrichFullName()
{
    var temp = name.Split(" ");
    name = $"First: {temp[0]}, Last: {temp[1]}"; // Side effect!
}

// ✅ Good - Pure function
public string SplitAndEnrichFullName(string name)
{
    var temp = name.Split(" ");
    return $"First: {temp[0]}, Last: {temp[1]}";
}

否定条件を避ける

// ❌ Bad
public bool IsDOMNodeNotPresent(string node) { }
if (!IsDOMNodeNotPresent(node)) { }  // Double negative!

// ✅ Good
public bool IsDOMNodePresent(string node) { }
if (IsDOMNodePresent(node)) { }

フラグ引数を避ける

// ❌ Bad - Flag indicates multiple responsibilities
public void CreateFile(string name, bool temp = false)
{
    if (temp)
        Touch("./temp/" + name);
    else
        Touch(name);
}

// ✅ Good - Separate methods
public void CreateFile(string name) => Touch(name);
public void CreateTempFile(string name) => Touch("./temp/" + name);

関数の引数を制限する（2つ以下）

// ❌ Bad
public void CreateMenu(string title, string body, string buttonText, bool cancellable) { }

// ✅ Good - Use object
public class MenuConfig
{
    public string Title { get; set; }
    public string Body { get; set; }
    public string ButtonText { get; set; }
    public bool Cancellable { get; set; }
}

public void CreateMenu(MenuConfig config) { }

条件文をカプセル化する

// ❌ Bad
if (article.state == "published") { }

// ✅ Good
if (article.IsPublished()) { }

デッドコードを削除する

// ❌ Bad
public void OldRequestModule(string url) { }  // Unused!
public void NewRequestModule(string url) { }

var request = NewRequestModule(requestUrl);

// ✅ Good - Delete unused code
public void RequestModule(string url) { }

var request = RequestModule(requestUrl);

---

SOLID 原則

単一責任の原則（SRP）

// ❌ Bad - Two responsibilities
class UserSettings
{
    public void ChangeSettings(Settings settings)
    {
        if (VerifyCredentials()) { /* ... */ }
    }

    private bool VerifyCredentials() { /* ... */ }  // Auth responsibility
}

// ✅ Good - Separated
class UserAuth
{
    public bool VerifyCredentials() { /* ... */ }
}

class UserSettings
{
    private readonly UserAuth _auth;

    public void ChangeSettings(Settings settings)
    {
        if (_auth.VerifyCredentials()) { /* ... */ }
    }
}

オープン・クローズドの原則（OCP）

// ❌ Bad - Must modify to extend
class HttpRequester
{
    public bool Fetch(string url)
    {
        if (adapterName == "ajaxAdapter")
            return MakeAjaxCall(url);
        else if (adapterName == "httpNodeAdapter")
            return MakeHttpCall(url);
        // Must add more else-if for new adapters!
    }
}

// ✅ Good - Open for extension, closed for modification
interface IAdapter
{
    bool Request(string url);
}

class AjaxAdapter : IAdapter
{
    public bool Request(string url) { /* ... */ }
}

class HttpRequester
{
    private readonly IAdapter _adapter;
    public bool Fetch(string url) => _adapter.Request(url);
}

リスコフの置換原則（LSP）

// ❌ Bad - Square breaks Rectangle behavior
class Square : Rectangle
{
    public override void SetWidth(double width) { Width = Height = width; }
}

// ✅ Good - Use abstraction
abstract class Shape
{
    public abstract double GetArea();
}

class Rectangle : Shape { /* ... */ }
class Square : Shape { /* ... */ }

インターフェース分離の原則（ISP）

// ❌ Bad - Robot can't eat but must implement
interface IEmployee { void Work(); void Eat(); }

class Robot : IEmployee
{
    public void Work() { /* ... */ }
    public void Eat() { /* Robot can't eat! */ }
}

// ✅ Good - Segregated interfaces
interface IWorkable { void Work(); }
interface IFeedable { void Eat(); }

class Human : IWorkable, IFeedable { /* ... */ }
class Robot : IWorkable { /* ... */ }

依存性逆転の原則（DIP）

// ❌ Bad - Depends on concrete types
class Manager
{
    private readonly Robot _robot;
    private readonly Human _human;
}

// ✅ Good - Depends on abstractions
class Manager
{
    private readonly IEnumerable<IEmployee> _employees;

    public Manager(IEnumerable<IEmployee> employees)
    {
        _employees = employees;
    }
}

コンストラクター依存関係のコードスメル（SRP の指標）

コンストラクターの依存関係が多すぎる場合、SRP 違反を示しています。

// ❌ Code Smell: 15 dependencies = too many responsibilities!
public class LicensePlateAppService : ApplicationService
{
    public LicensePlateAppService(
        IRepository<LicensePlate, Guid> licensePlateRepository,
        IRepository<LicensePlateWithoutTag, Guid> licensePlateWithoutTagRepository,
        IRepository<ASN, Guid> asnRepository,
        IRepository<Project, Guid> projectRepository,
        IRepository<Tag, Guid> tagRepository,
        IRepository<SKU, Guid> skuRepository,
        IRepository<Customer, Guid> customerRepository,
        IRepository<LicensePlateHold, Guid> licensePlateHoldRepository,
        IRepository<LicensePlateLocation, Guid> licensePlateLocationRepository,
        IRepository<Location, Guid> locationRepository,
        IWarehouseAppService warehouseAppService,
        IWarehouseOwnerAppService warehouseOwnerAppService,
        IBlobContainer<BulkUpdateLPExcelFileContainer> fileContainer,
        LicensePlateService.LicensePlateServiceClient licensePlateServiceClient,
        CommonDependencies<LicensePlateAppService> commonDependencies)
    { }
}

// ✅ Good: Split by responsibility
public class LicensePlateAppService { }      // CRUD only (~5 deps)
public class LicensePlateBulkService { }     // Bulk imports (~4 deps)
public class LicensePlateEventPublisher { }  // Events (~3 deps)

依存関係数のガイドライン：

依存関係	状態	対応
1-5	✅ 通常	許容範囲
6-8	⚠️ 警告	分割できる箇所がないか確認
9+	❌ コードスメル	リファクタリングが必要です。クラスの責任が多すぎます

リファクタリングの戦略：

1. サービスの抽出 - 関連する処理を専用サービスに移動する
2. ファサードパターン - 関連する依存関係をファサードの背後にまとめる
3. ドメインイベント - 直接呼び出す代わりに、パブリッシュ／サブスクライブで疎結合にする
4. メディエーターパターン - MediatR を使って直接的な依存関係を減らす

---

エラー処理

throw ex を使わない

// ❌ Bad - Loses stack trace
catch (Exception ex)
{
    logger.LogError(ex);
    throw ex;  // Stack trace lost!
}

// ✅ Good - Preserves stack trace
catch (Exception ex)
{
    logger.LogError(ex);
    throw;  // Rethrows with original stack
}

// ✅ Also Good - Wrap with inner exception
catch (Exception ex)
{
    throw new BusinessException("Operation failed", ex);
}

捕捉したエラーを無視しない

// ❌ Bad - Silent swallow
catch (Exception ex) { }  // Never do this!

// ✅ Good - Handle or propagate
catch (Exception ex)
{
    _logger.LogError(ex, "Operation failed");
    throw;  // Or handle appropriately
}

複数の catch ブロックを使う

// ❌ Bad - Type checking in catch
catch (Exception ex)
{
    if (ex is TaskCanceledException) { /* ... */ }
    else if (ex is TaskSchedulerException) { /* ... */ }
}

// ✅ Good - Separate catch blocks
catch (TaskCanceledException ex)
{
    // Handle cancellation
}
catch (TaskSchedulerException ex)
{
    // Handle scheduler error
}

---

コメント

位置を示すマーカーやリージョンを避ける

// ❌ Bad
#region Scope Model Instantiation
var model = new Model();
#endregion

#region Action setup
void Actions() { }
#endregion

// ✅ Good - Let code speak
var model = new Model();

void Actions() { }

コメントアウトしたコードを残さない

// ❌ Bad
DoStuff();
// DoOtherStuff();
// DoSomeMoreStuff();

// ✅ Good - Use version control
DoStuff();

ビジネスロジックの複雑さだけにコメントする

// ❌ Bad - Obvious comments
var hash = 0;  // The hash
var length = data.Length;  // Length of string

// ✅ Good - Explains WHY, not WHAT
// Using djb2 hash for good speed/collision tradeoff
hash = ((hash << 5) - hash) + character;

---

クイックリファレンスチェックリスト

コードレビューチェックリスト

• 命名：意味があり、発音しやすく、ハンガリアン記法を使っていない
• 関数：単一責任、引数は3つ未満、フラグなし
• 変数：マジックストリングなし、早期リターン、ネストは2段まで
• SOLID：具象クラスよりインターフェース、小さく責務を絞ったクラス
• 依存関係：コンストラクターの依存関係は8つ未満（SRP の指標）
• エラー処理：throw ex なし、エラーを黙って捕捉しない、具体的な例外型を使う
• コメント：リージョンなし、デッドコードなし、「なぜ」を説明する

---

参考資料

• references/solid-principles.md：SOLID の詳しい例
• references/async-patterns.md：Async/await のガイドライン
• references/editorconfig-template.md：.editorconfig テンプレート

ソース：clean-code-dotnet