連載Index(読む順・公開済(リンク)はここが最新): S00_門前の誓い_総合Index
レビューで地雷になりやすい箇所はだいたい同じ。名前が雑。例外が消える。公開範囲(スコープ)が広い。依存方向が揺れる。
症状は軽く見えても、原因が追えないコードは「レベルが低い」と思われやすい。直しの依頼が増え、手戻りも増えて自己嫌悪に陥る。
命名の雑さは意味の取り違えを生み、可読性と保守性を落とす。
Flg はダサい。Util は……まぁ現場では使うこともある。だが、置き場にした瞬間に意図が死ぬ。
例外とログが薄いのも恥ずかしい。障害対応で再現できずに詰むし、次に同じ事故が起きる。
ここでは門前の最小ラインとして、(1)名前で役割を固定する、(2)例外を消さずに証拠を残す、(3)公開範囲(スコープ)と依存方向を絞って影響範囲を閉じる、を揃える。
目次
このページのゴール
- 命名の揺れを止め、意図が伝わらない箇所を減らす(可読性/保守性の底上げ)
- 例外を消さず、障害時の原因追跡を成立させる
- ログ/例外/返り値の責務分担を揃え、現場の再現性を上げる
- 公開範囲(スコープ)を小さく保ち、依存関係の暴走を抑える
- レビュー指摘を「好み」から「事故の形」へ寄せる
門前チェック(最短)
- ファイル名と主要型名が一致している
- クラス名と役割(責務)が一致している(名前だけ先行していない)
- namespace とディレクトリが揃っている
- bool が true の意味で読める(Flg を使わない)
- 例外が握り潰されていない(再 throw は throw;)
- public の範囲が必要最小限(公開フィールド/可変 static/変更可能コレクションの公開が無い)
- using/Dispose が抜けていない(event の登録解除(イベントを外す)漏れを含む)
- 金額/小数の値は型が適切(float/double/decimal 等)で、丸め方針が境界で固定されている
掟の本文(コーディング規約の全文)
一定品質のコード作成とバグ低減に繋がる最小セットを、規約として全文掲載する。
同じ単語でも意味が揺れる箇所(例: interface と abstract class)は、規約側で定義と使い分けを先に固定する。
以降は命名→コメント→実装の順で並べる。
命名規則
(1) ファイル名
ファイル名はパスカル記法とし、公開される型(class/struct/record/enum)の主要型名とファイル名は同名とします。
partial クラスや関連型を同一ファイルにまとめる場合は「中心となる型名」をファイル名にします。
例: public class ValueObject は ValueObject.cs に入れる。
[パスカル記法]
- 先頭を大文字
- それ以外は小文字
- 言葉の区切りは大文字
ApplicationInfo
AsyncTask
TestBean
ValueObject
(2) 名前空間(namespace)
名前空間は「会社/組織の逆ドメイン + 製品/プロジェクト + レイヤ」を基本とします。
(例: Company.Product.Feature)
ディレクトリ構成は名前空間と揃えるのが一般的です。
Company.Product.ProjectName
System.Collections.Generic
(3) クラス名 / 構造体名 / レコード名
(1) と同じくパスカル記法とする。SOLID 原則にしたがって命名するのがよい。
型名は「名詞」または「名詞句」を基本とし、責務が曖昧な Manager / Util 連発は避けます。ダサいだけでなく、役割が読めない合図になります。
命名は「見た目」ではなく設計そのものです。
役割が読めない名前は、レビューで説明が必要になり、保守で事故る。ここが雑だと一気にダサく見えます。
record について補足:record は C# 9.0 で導入された型で、値のように扱うデータ(等価性/with/分解)を作りやすくします。
record でも参照型のフィールドを持てば「中身は共有」になり得るため、コピーの章(シャロー/ディープ)とセットで扱います。
長い名前になりそうな時の縮め方(雑に省略しない):
- スコープで短くする(狭いスコープなら短くても読める)
- 冗長語を落とす(Manager/Util/Info/Data 連発を削る)
- 同じ単語の重複を落とす(型名に含まれる語を変数名で繰り返さない)
- 意味の塊で並べ替える(「目的語 + 役割」など一定の順序にする)
- 型で語らせる(型名に寄せ、変数名を短くする)
(4) 例外クラス名
標準の例外クラスでは表現できない場合に独自例外を作成する際は、最後を Exception とします。
例外名からエラー種別が判断できる名前にします。
StreamReaderException
InvalidConfigurationException
(5) インターフェイス名
インターフェイス名は I プレフィックス + パスカル記法 を基本とします。
IRepository
INotificationSender
能力(capability)を表す場合は .NET の慣例に合わせ、IDisposable / IAsyncDisposable / IComparable 等のように「形容詞的な命名」にします。
(6) 抽象クラス名
抽象クラス名は、役割が分かる名前を先に置き、必要なら Base を付けます。
前に付ける(BaseXxx)か後ろに付ける(XxxBase)かは流派が分かれます。現場では XxxBase の方が見かけることも多い。
どちらでもよいが、プロジェクト内で統一しないとダサく見える(混在は意図が読めない)。
// OK: 後ろに Base (統一されているなら読みやすい)
ControllerBase
RepositoryBase
// OK: 前に Base (統一されているなら読みやすい)
BaseController
BaseRepository
ポイントは「Base で逃げない」こと。Base は共通化の入口になりやすく、責務が溶けると保守が崩れます。
Base に寄せる前に、何が共通で何が違うかを先に言語化します。
(7) メソッド名
メソッド名は パスカル記法 とし、[動詞]+[目的語] を意識します。
ToString()
CreateUser()
FindDirectory()
AddValue()
(8) ファクトリメソッド
インスタンス生成を行う場合、意図が分かる CreateXxx / FromXxx / Parse / TryParse を基本とします。
NewXxx を乱用せず、C# の慣例(new 演算子・コンストラクタ)と競合しない名称にします。
public static User Create(string name)
public static DateTime Parse(string s)
public static bool TryParse(string s, out DateTime value)
(9) コンバートメソッド
別オブジェクトへ変換する場合は ToXxx / AsXxx / ConvertToXxx 等を使用します。
ToXxx は「情報落ちが無い / 例外を投げない」等の方針をプロジェクトで統一します。
string ToJson()
UserDto ToDto()
bool TryConvertTo(out UserDto dto)
(10) プロパティ
C# では getter/setter ではなく プロパティ が基本です。
Java の getXxx()/setXxx() に寄せると、API が冗長になり「C# らしさが無い」「レベルが低い」と思われやすい。
まずは自動実装プロパティが標準です。
// OK: 大抵これで足りる
public int Value { get; private set; }
public string Name { get; init; } = "";
中身を書き分けるのは「理由がある時だけ」です。
private int _value;
// OK: 検証/正規化/副作用(通知)など、明確な理由がある時だけ
public int Value
{
get { return _value; }
private set
{
if (value < 0) throw new ArgumentOutOfRangeException(nameof(value));
_value = value;
// 必要ならここで通知や派生値更新
}
}
メソッドとして分けるのは「操作」の意味を持たせたい時だけです(単なる取得/設定ならプロパティで十分)。
// OK: 意味が「操作」ならメソッド
public void ResetValue() => _value = 0;
(11) boolean を返却するメソッド
bool を返却するメソッドは「true の意味」が読みやすい名称にします。
通常は true 側の状態を名称にし、Flg などは使用しません。Flg 連発はレベルが低いと思われやすい。
- is + 形容詞
- can + 動詞
- has + 名詞
- should + 動詞
- 三単現動詞 + 名詞
bool IsEmpty();
bool CanRetry();
bool HasChanges();
bool ContainsItem(object item);
bool ShouldRetry();
悪い例:
bool Empty(); // 状態取得のメソッド名としてはNG(空にする処理に見える)
(12) 定数名
公開 API として公開する const / static readonly は パスカル記法 を基本とします(.NET の慣例)。
private 定数でも同方針で統一し、プロジェクト事情で UPPER_SNAKE_CASE を採用する場合は全体で揃えます。
public const int MaxRetryCount = 3;
private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(30);
(13) 変数名
ローカル変数・引数はキャメルケース(lowerCamelCase)とします。
private フィールドは _camelCase を基本とします(チーム方針で統一)。
string value;
int count;
DateTime startDate;
DateTime endDate;
private readonly string _connectionString;
用途が判断できない名前は避けます。
// NG 例
string info;
string data;
string temp;
string str;
string buf;
(14) enum
enum 名・メンバー名はパスカル記法とします。[Flags] を付ける場合は複数形/ビット表現を意識します。
public enum Color
{
Red = 0,
Green = 1,
Blue = 2,
}
(15) boolean 変数 / プロパティ
true の意味が読みやすい名称にします((11) と同様)。xxxFlg は厳禁です。ダサい上に、true/false の意味を読み違える事故が起きます。
bool isEmpty; // true の場合、空である
bool canGet; // true の場合、取得可能である
bool hasChanged; // true の場合、変更後である
bool containsKey; // true の場合、Key が含まれている
(16) 英語と日本語
命名は英語を基本とします。
日英対応用語辞書(用語集)を作成し、プロジェクト全体で揺れを減らすと効果的です。
(17) 名前の対称性
クラス名・メソッド名は英語の対称性に注意し、ペアが分かる命名にします。
| 対称 | 英名 |
|---|---|
| 追加/削除 | add/remove |
| 追加/削除 | insert/delete |
| 取得/設定 | get/set |
| 開始/停止 | start/stop |
| 開始/終了 | begin/end |
| 送信/受信 | send/receive |
| 最初/最後 | first/last |
| 取得/解放 | get/release |
| 配置/取得 | put/get |
| 上げる/下げる | up/down |
| 表示する/隠す | show/hide |
| 元/先 | source/target |
| 元/先 | source/destination |
| 開く/閉じる | open/close |
| 増加/減少 | increment/decrement |
| ロックする/ロック解除 | lock/unlock |
| 古い/新しい | old/new |
| 次/前 | next/previous |
(18) ループカウンタ
for 文等で頻繁に利用されるカウンタは i, j, k … を使用します。
スコープが広い場合は index, count など意味のある名前を付けます。
for (int i = 0; i < list.Count; i++)
{
for (int j = 0; j < list.Count; j++)
{
// ...
}
}
for (int attempt = 1; attempt <= MaxRetryCount; attempt++)
{
// ...
}
(19) スコープが狭い名前
スコープが狭い場合は、型名の略称を使ってもよい(ただし読み手が理解できる範囲に限る)。
var sb = new StringBuilder();
var dt = DateTime.Now;
(20) 引数
引数は意図が分かる名前にします。
// 悪い例: Copy(s1, s2)
// 良い例: Copy(source, destination)
コメント
(21) コメントについて
コメントはクラス、メソッド、重要な分岐、複雑な処理に対して「なぜそうするか」が分かるように記載します。
処理の逐語説明ではなく、仕様・制約・意図・注意点を優先します。
(22) コメントの種類
C# のコメントは主に以下です。
/* */ // 複数行コメント
// // 1行コメント
/// <summary> // XML ドキュメントコメント
(23) 長いコメント
複数行コメントは「概要→詳細」の順で書くと理解しやすいです。
/*
* データ削除処理
* 以下の条件の場合にデータを削除する。
* →期限が切れている
* →無効になっている
*/
(24) // と /* */
1 行は //、複数行は /* */ を基本とし、読みやすさで選択します。
int index = -1; // -1 は invalid な値を意味する
可能なら定数化します。
private const int InvalidIndex = -1;
int index = InvalidIndex;
(25) XML ドキュメントコメント
公開 API(public / protected / internal のうち利用範囲が広いもの)は XML ドキュメントコメントを付けます。
生成されたドキュメントは IDE の IntelliSense に反映され、仕様共有に有効です。
(26) XML ドキュメントコメント記載方法
「何をするか」「引数は何か」「戻り値は何か」「例外は何か」だけを短く揃えます。
凝った例より、普段のメソッドで読める形の方が現場で効きます。
/// <summary>文字列を整数に変換する。</summary>
/// <param name="text">入力文字列。</param>
/// <param name="value">変換結果。</param>
/// <returns>変換できた場合は true。</returns>
/// <exception cref="ArgumentNullException">text が null の場合。</exception>
public static bool TryParseInt(string? text, out int value)
{
if (text is null) throw new ArgumentNullException(nameof(text));
return int.TryParse(text, out value);
}
ポイント:
- summary は 1 行で十分(長文にしない)
- param/returns は「何が入って何が出るか」だけ
- 例外を投げるなら exception を書く(黙って投げるのは恥ずかしい)
(27) クラスコメント
クラス定義の直前に /// <summary> で機能概要を記載します。
1 行目は短く端的にまとめ、必要に応じて詳細・使用例を追記します。
/// <summary>スタックを表現するクラス。</summary>
/// <remarks>
/// 先入れ後出し(LIFO)のデータ構造。
/// </remarks>
public class Stack<T>
{
// ...
}
コーディング規約
(28) アクセス修飾子
クラス、メソッド、変数にはアクセス修飾子を設定します。
| アクセス修飾子 | 概要 |
|---|---|
| private | 現在の型からだけアクセスできる |
| protected | 現在の型と派生型からアクセスできる |
| internal | 同一アセンブリ内からアクセスできる |
| public | すべてのアセンブリからアクセスできる |
| protected internal | 同一アセンブリ or 派生型からアクセスできる |
| private protected | 同一アセンブリ内の派生型からアクセスできる |
(29) using
using は未使用のものを残さない。IDE の整理機能で最適化します。
グローバル using 等の機能を使う場合も、プロジェクト内で統一します。
using System;
using System.Collections.Generic;
using System.IO;
(30) コーディングスタイル
プロジェクトでフォーマッター(.editorconfig、ReSharper 等)が設定されている場合はそちらに従います。一般的な指針は以下です。
/* COPYRIGHT ... */ // --- ①
using System; // --- ②
using System.Collections.Generic;
namespace Company.Product.Util // --- ③
{
/// <summary>Stack を表現するクラス。</summary> // --- ④
public class Stack<T> // --- ⑤
{
/// <summary>要素を追加する。</summary> // --- ⑥
public void Push(T item) // --- ⑦ ⑧
{
if (item is null) // --- ⑨
{
throw new ArgumentNullException(nameof(item));
}
else // --- ⑩
{
// ...
}
}
public T Pop()
{
// ...
return default; // --- ⑪
}
}
}
①ファイル先頭にコピーライトが入ることがある。
②次に using、1行空けて namespace。
③ディレクトリと namespace を揃える。
④公開クラスには XML コメント。
⑤{ は基本同一行(統一すること)。
⑥メソッドのコメントも同様。
⑦インデントは 4 SPACE。
⑧メソッド定義の { も同一行(統一)。
⑨キーワードと ( の間はスペース 1 つ。演算子の両側にスペース。
⑩if/else の {} 位置は統一。
⑪return 値を不要に () で囲まない。
(31) 長い行
一行は横スクロール無しで読める範囲を目安とし、超える場合は行分割を検討します。
分割の指針は、(1) ローカル変数化、(2) カンマで改行、(3) 優先度の低い演算子の前で改行。
(32) 長い宣言行
宣言が長い場合、(1) 継承/実装、(2) 引数リスト、(3) ジェネリクス制約 などで改行します。
public class LongNameRepository<T>
: BaseRepository<T>,
IDisposable
where T : class
{
public void LongMethodSignature(
int a, int b, int c,
int d, int e, int f)
{
// ...
}
}
(33) 同じ処理を書かず、可能な限り共通化
同一処理が複数箇所に存在すると保守性・テスト工数が悪化します。
共通化(メソッド化、共通クラス化、拡張メソッド化)を検討します。
(34) インターフェイスと抽象クラスの使い分け
- インターフェイス:外部に公開する契約(振る舞い)を定義
- 抽象クラス:共通実装(骨組み)やテンプレートメソッドを提供
多重継承ができない点を意識し、抽象クラスの肥大化を避けます。
(35) public フィールド
基本的にフィールドは private とし、プロパティを公開します。
例外として DTO など「不変で整合性を崩さない」ケースで public フィールド/init を許容する場合は、プロジェクトで合意します。
(36) 複数の値を返却する場合は型を作る
出力引数(out/ref)で値を変更する設計は濫用しない。
複数戻りが必要な場合は record/struct/class または ValueTuple を検討します。
public (int X, int Y) CalculatePoint() => (x, y);
// または
public sealed record Point(int X, int Y);
(37) 定数だけのインターフェイスは作らない
インターフェイスは約束であり、定数置き場にはしません。
定数は static class / enum / readonly 等で表現します。
(38) 初期化
ローカル変数は使用前に必ず代入が必要です。
フィールドは既定値を持ちますが、readonly フィールドはコンストラクタ等での初期化が必要です。
(39) static 変数使用時の注意
static は全インスタンスで共有されます。
可変 static はスレッド安全性・テスト容易性を悪化させるため、基本は static readonly とし、必要時のみ厳格に同期します。
(40) メソッド定義について
メソッドは「1つの事」を行うように実装します。
長大メソッドは分割し、再利用性とテスト容易性を高めます。
(41) Equals と GetHashCode のオーバーライド
Equals(object) をオーバーライドする場合は GetHashCode() もオーバーライドします。
必要に応じて IEquatable<T> を実装し、==/!= を定義する場合は整合性を保ちます。
(42) クローン(コピー)について
ICloneable は「浅い/深い」の意味が曖昧になりやすいため濫用しません。
コピーが必要な場合は以下を優先します。
- 不変(immutable)設計にする
-
recordのwith式を使う - コピーコンストラクタ /
Copy()メソッドを明示する
(43) ディープコピーについて
参照型を含むオブジェクトを複製し、参照先も独立させるコピーです。
コピー元/先の変更が相互に影響しない状態を作ります(「別物」を作る)。
初心者が詰むポイントは「外側だけ new しても、中身が同じ参照のまま」という落とし穴です。
// 例: List<Child> を持つ Parent を deep copy したい
public sealed class Parent
{
public List<Child> Children { get; } = new();
}
public sealed class Child
{
public int Id { get; init; }
}
// OK: 中身も新しく作る(深い所まで複製)
public static Parent DeepCopy(Parent src)
{
var dst = new Parent();
foreach (var c in src.Children)
{
dst.Children.Add(new Child { Id = c.Id });
}
return dst;
}
ディープコピーはコストが高いので「本当に必要か」を先に決めます。
必要な場面は「変更が混ざると事故る」場面(キャッシュ、履歴、Undo、並列処理のスナップショット等)です。
(44) シャローコピーについて
オブジェクト自身は複製されますが、参照型の参照先は共有されます。
「箱だけコピーして、中身は同じ物を指す」状態です。意図せぬ副作用が起きやすい。
public sealed class Parent
{
public List<int> Items { get; init; } = new();
}
// NG: Items は同じ List を共有する(片方の変更がもう片方に混ざる)
public static Parent ShallowCopy(Parent src)
{
return new Parent { Items = src.Items };
}
record の with も「参照型プロパティの中身は共有」になり得ます。
public record Config(List<string> Servers);
// 注意: Servers(List) は共有される
var a = new Config(new List<string> { "A" });
var b = a with { }; // 見た目はコピーでも、中身は同じ参照
シャローで十分な場面は「中身を不変として扱える」場合だけです。
不変にできないなら、ディープコピーか設計の見直し(不変化/コピー不要化)を優先します。
(45) デフォルトコンストラクタ
コンストラクタを 1 つでも定義すると、暗黙の引数なしコンストラクタは生成されません。
シリアライザ等で必要な場合は明示的に用意します。
(46) オブジェクトの同値比較
参照同一性と値の同値を混同しない。
== は型により意味が異なります(string は値比較)。基本は Equals/IEquatable<T> を利用します。
IEquatable<T> とは:同値比較(Equals)を型として定義し、比較の意図と性能を安定させるための仕組みです。
同値比較が多い型(ValueObject など)で効きます。
null 事故を減らす比較の型:
-
categoryCode.Equals("A")はcategoryCodeが null だと落ちる(レベルが低いと思われやすい) -
string.Equals(categoryCode, "A", StringComparison.Ordinal)は null でも落ちず、比較方法も明示できる
// OK: null 安全 + 比較方法が明示される
if (string.Equals(categoryCode, "A", StringComparison.Ordinal))
{
// ...
}
// OK: パターンマッチでも書ける(null 安全)
if (categoryCode is "A")
{
// ...
}
文字列比較は文化圏(カルチャ)で結果が変わることがあるため、比較方法を明示します。
迷ったら StringComparison.Ordinal を基準にし、UI 表示向けの比較だけ CurrentCulture 等を使い分けます。
(47) 宣言と初期化
ローカル変数は「使う直前に宣言と初期化」を行い、スコープを短くします。
(48) ローカル変数のスコープはできるだけ短くする
同名の再利用よりも、必要箇所で宣言する方が可読性が上がります。
(49) 変数への再代入を避ける(できるだけ)
再代入は状態把握を難しくします。
可能なら readonly / 不変型 / LINQ / 早期 return を活用します(ただし性能要件がある場合は例外)。
(50) if/while 条件中の "="
条件式中の代入はバグの温床です。
bool の代入はコンパイル上成立するため、原則禁止とし「代入は別行」にします。
// NG 例
if (isValid = Validate()) { /* ... */ }
// OK 例
isValid = Validate();
if (isValid) { /* ... */ }
(51) 大小比較演算子
大小比較は「向き」と「書き方」をプロジェクト内で統一します。
例:index < count の形を基準にし、逆向き(count > index)を混在させない(読み間違いを減らすのが目的)。
(52) キャスト
キャストは安全性を意識します。
- 参照型:
as+ null チェック / パターンマッチis - 数値:オーバーフローに注意(
checkedの利用検討) - 失敗が起こり得る場合:例外に頼らず
TryXxxパターンを検討
(53) キャストではない型変換
文字列⇔数値は Parse/TryParse を使用します。
例外でフロー制御しない。
if (int.TryParse(text, out var value))
{
// value を使用
}
(54) アップキャスト
派生 → 基底は暗黙的に安全に行えます。
Stream s = new MemoryStream();
(55) ダウンキャスト
基底 → 派生は危険です。as / パターンマッチで安全に判定します。
if (obj is MyType t)
{
// t を使用
}
(56) 例外
例外は握りつぶさず、原因追跡できる情報(ログ、inner exception)を残します。
catch するなら「対処できる場合のみ」。再 throw は throw; を使用しスタックを保持します。
よくある NG:
-
catch (Exception) {}の空 catch(情報を捨てるだけで恥ずかしい。レベルが低い実装に見えます) -
throw ex;(スタックを壊す。後から追えない)
(57) 独自例外クラス
独自例外を作成する場合は Exception を継承し、標準的なコンストラクタを実装します。
(必要であれば InnerException を渡すコンストラクタも用意)
(58) メソッド内での引数の値変更
原則として引数を変更しない(特に参照型の内部状態)。
必要な場合のみ ref/out を使い、呼び出し側に副作用を明示します。
(59) メソッド引数の名前
読みやすさ優先。フィールドと衝突する場合は this. を利用し、引数に _ 等を付けない。
(60) コレクション
ジェネリックコレクションを基本とします。
外部に公開する型は IReadOnlyList<T> / IEnumerable<T> 等を検討し、不要な変更操作を公開しない。
(61) ループの種類について
ループは「用途」と「性能」を同時に意識します。書き分けが雑だと、読み手にダサく見えます(意図が読めない)。
用途の整理:
-
for:回数/インデックスが明確(配列/List<T>の走査、範囲指定、逆順など) -
foreach:列挙(要素を順に読む)。インデックスが不要な時に読みやすい -
while:条件で回す(終了条件がループ本体で変わる、待機/リトライなど) -
do-while:最低 1 回は実行が要件(入力ループなど)
注意点:
-
foreach中にコレクションを書き換えると例外になり得る(構造が変わるため)
変える必要があるなら、コピーしてから回す/forにする/別リストへ積む。
性能の目安(迷った時の判断材料):
- 配列や
List<T>を単純に回すならforが最も素直で速いことが多い -
foreachは対象がList<T>等なら最適化されやすいが、IEnumerable<T>経由だと列挙子生成が入ることがある - LINQ(
Where/Select) は読みやすい反面、割り当て/列挙コストが増えやすい(ホットパスは注意)
速度が気になる箇所は「体感」ではなく計測で決めます。
まずは読みやすさ優先、ホットパスだけ測って詰めます(最初から全部速さに寄せるのは恥ずかしい)。
(62) メモリリーク
主な原因例:
- event の登録解除漏れ(イベントを外し忘れる)
- Timer / static 参照がオブジェクトを掴み続ける
-
IDisposableの破棄漏れ(ファイル/ソケット/DB/ウィンドウハンドルなど)
なぜ起きるか:
- .NET の GC は「メモリ」を回収するが、参照が残る限り回収できない
- event は「購読側ではなく発行側が参照を持つ」ため、解除しないと生存し続けることがある
-
IDisposableは GC 任せにすると解放タイミングが遅れ、ハンドル枯渇など別の事故になる
対策の型:
- event を登録したら、寿命の終わり(Dispose/Closed/Unloaded 等)で必ず外す
- 長寿命(static/シングルトン)が短寿命(UI/画面)を掴まない設計にする
-
usingで確実に閉じる(後ろの「ガベージコレクション」の章も参照)
「イベントを外す」までセットで書けていると、現場では一気にレベルが上がって見えます。
(63) 自動クローズ(using)
破棄が必要なリソースは using(using var / await using を含む)を使用します。
using var stream = File.OpenRead(path);
// stream はスコープ終了時に Dispose
(64) ガベージコレクション
GC(ガベージコレクション)は自動ですが、IDisposable は別問題です。
GC が回収するのは基本的に「マネージメモリ」です。ファイル/ソケット/DB 接続/ウィンドウハンドル等の「外部資源」は別で、放置すると枯渇することがあります。
つまり:
- GC 任せ:いつ解放されるか分からない(タイミングが遅い)
-
Dispose/using:ここで確実に閉じる(タイミングが決まる)
// OK: ここで確実に閉じる
using var stream = File.OpenRead(path);
// stream を使う
よくある勘違い:
- 「GC があるから Dispose は不要」→ 間違い。外部資源は GC と別レーン
- 「finalizer があるから平気」→ 期待しない。呼ばれるタイミングは保証されない
WinForms はハンドル/イベント/タイマーで掴み合いが起きやすい。
Dispose/解除が揃っていると、障害対応での再現性が上がり「レベルが低い」扱いを回避できます。
(65) null 参照(NullReferenceException)を出さない書き方
null は「仕様としてあり得るのか」を先に決めます。あり得るなら、落ちない形で書きます。
obj.Method() を無造作に呼ぶのは、外から見ると雑でレベルが低いと思われやすい(恥ずかしい落ち方をする)。
代表パターン:
- ガード節で早めに弾く(ArgumentNullException)
-
?./??/ パターンマッチを使う - 文字列比較は
string.Equalsを使う(呼び出し側が null でも落ちない)
if (item is null) throw new ArgumentNullException(nameof(item));
var name = item?.Name ?? ""; // 仕様に合わせる
(66) 範囲外アクセス(IndexOutOfRangeException)を出さない書き方
配列/リスト/文字列のインデックスは、境界チェックが無いと一発で落ちます。
落ち方が派手で恥ずかしい上に、ログが薄いと原因も追えません。
基本の型:
- 先に長さを確認する
- 取得系は Try 系を使う(辞書は TryGetValue)
- どうしても不明なら「無ければ何もしない」を明示する
if (0 <= index && index < list.Count)
{
var x = list[index];
}
if (dict.TryGetValue(key, out var value))
{
// value を使う
}
(67) コレクションを公開する時は「読むだけ」の型で返す
List<T> をそのまま返すと、呼び側が勝手に追加/削除できて事故ります。
外からの変更を許すのはレベルが低く見えやすいポイントです(意図が読めない)。
-
IReadOnlyList<T>:インデックス付きで読むだけ(読む側は変更できない) -
IEnumerable<T>:列挙して読むだけ(要素数や再列挙の特性に注意)
private readonly List<Item> _items = new();
// OK: 読むだけで返す
public IReadOnlyList<Item> Items => _items;
ここまで揃うと公開 API の筋が通り、レベルが高い設計に見えます。
(68) 例外メッセージは「何が」「どこで」を残す
例外を投げるなら、メッセージに最低限の文脈を入れます。
"Invalid" だけの例外は、障害対応で役に立たずダサい(原因が追えない)。
throw new InvalidOperationException($"状態が不正。state={state}, id={id}");
(69) 数値型(float/double/decimal/BigDecimal)の使い分け
数値は「型を間違える」と、丸め誤差や比較ミスで静かに事故ります。
適切な型と丸め方針が揃っているだけで、現場の信頼が上がります。
基本方針(迷ったらこれ):
-
金額/税率/歩合/人が読む小数:
decimal(10 進)。0.1 等を正確に持てる。丸め(何桁・どの方式)を境界で固定する -
一般的な計算(統計/幾何/物理/座標):
double。精度と速度のバランスが良い(既定の浮動小数リテラルも double) -
大量データ/メモリ優先/グラフィックス/ML:
float。精度は落ちるがメモリと帯域が効く -
任意精度の 10 進(いわゆる BigDecimal):C# 標準には
BigDecimalが無い
必要なら「最小単位の整数で持つ(円/銭/ポイント等)」「BigInteger+ スケール」「ライブラリ採用」を検討する
精度と扱える桁(目安):
| 型 | 方式 | 有効桁(目安) | 範囲(目安) | 向いている用途 |
|---|---|---|---|---|
float |
IEEE 754 2 進(32bit) | 約 7 桁 | 約 1E±38 | 大量配列、グラフィックス、ML |
double |
IEEE 754 2 進(64bit) | 約 15〜16 桁 | 約 1E±308 | 一般計算、統計、座標、幾何 |
decimal |
10 進(固定小数) | 約 28〜29 桁 | 約 1E±28 | 金額、税率、業務の割合 |
| BigDecimal相当 | 任意精度 10 進 | 任意 | 任意 | 極端な精度が必須な領域(通常は過剰) |
注意点(ここで詰みやすい):
-
float/doubleは 2 進小数。0.1など「10 進の小数」が正確に表現できず、==比較は事故りやすい
比較が必要なら許容誤差(ε)を使う/丸めてから比較する -
decimalは 10 進で金額向きだが、丸めが曖昧だと「計算途中の誤差」が仕様差になる
丸め桁数 と 丸め方式(例:MidpointRounding.AwayFromZero/ToEven)を規約化し、境界(入出力/永続化/表示)で固定する - 浮動小数リテラルは既定で
double。floatは1.0f、decimalは1.0mのようにサフィックスで意図を固定する(混ざると暗黙変換で精度が落ちる)
// NG: double の == は事故りやすい
if (a == b) { /* ... */ }
// OK: 許容誤差で比較(用途に応じて eps は調整)
static bool NearlyEqual(double x, double y, double eps = 1e-10)
=> Math.Abs(x - y) <= eps;
まとめ
共通ルールを守ることで一定品質のコード作成とバグ低減に繋がります。
可読性・保守性の観点でも効果が高い。規約をベースにした実装を前提とする。
判例(OK/NG)
| 観点 | OK例 | NG例 | 理由 | レビューで見る所 |
|---|---|---|---|---|
| ファイル名 |
ValueObject.cs に public class ValueObject
|
ValueObject2.cs に主要型が散る |
定義場所を探す時間と、この型の役割を追う時間が増える | ファイル名と主要型名の一致 |
| namespace |
Company.Product.Feature + ディレクトリ一致 |
namespace とフォルダが無関係 | 依存方向が見えず、移動/分割が崩れる | namespace とディレクトリ |
| bool 命名 |
IsEmpty() / CanSave() / HasItems
|
Empty() / xxxFlg
|
true/false の誤読で分岐事故が起きる | bool の命名規則 |
| 例外処理 | 対処+ログ+throw;
|
catch (Exception) {} / throw ex;
|
原因追跡不能、スタック破壊 | catch の範囲、再 throw |
| 返り値と例外 | 失敗が通常系なら TryXxx
|
期待される失敗でも例外依存 | 例外でフロー制御になり性能/可読性が落ちる | Parse/TryParse、TryXxx |
| 定数の置き場 |
static class Constants / enum
|
定数だけの interface | 境界が曖昧になり依存が拡散する | 定数の配置 |
| 公開範囲 | private + 不変条件を守る API | public field を無造作に増やす | 不変条件が壊れ、変更影響が読めなくなる | アクセス修飾子、公開フィールド |
| static |
static readonly を基本 |
可変 static を共有 |
スレッド事故/テスト不能/副作用拡大 | static の用途と可変性 |
| コレクション公開 |
IReadOnlyList<T> / IEnumerable<T>
|
List<T> を public で返す |
呼び側が勝手に変更し不具合が伝播 | 公開 API の型 |
| リソース破棄 |
using / 明示 Dispose |
IDisposable を放置 |
ハンドル枯渇/リークで障害が再現する | using/Dispose、イベント解除 |
レビュー観点
| 観点 | ありがちな見落とし | 影響パターン | 指摘コメント例(直球禁止) |
|---|---|---|---|
| 命名 |
Manager/Util で責務が溶ける |
機能追加で肥大化し分割不能 | 「役割が型名から確定しない。名詞句で責務が見える形が安全」 |
| bool |
Flg で true の意味が不明 |
条件式の誤読で逆分岐 | 「true 側が読める命名だと条件式の誤読が減る」 |
| 例外 | catch が広い/空 | 障害時に原因が追えない | 「対処が無い catch は情報が消える。対処と追跡情報がセットだと成立する」 |
| 例外再送出 | throw ex; |
スタックが切れて根が追えない | 「再送出はスタック保持が重要。throw; の方が追跡が繋がる」 |
| 公開範囲 | public が広い | 依存が増え変更が怖くなる | 「公開範囲が広いほど影響範囲が読めなくなる。スコープを絞ると変更が安全」 |
| コレクション | 変更可能な型を返す | 呼び側が壊して伝播 | 「変更操作まで公開している。読み取り専用の型だと影響が閉じる」 |
| 破棄 | Dispose/解除が漏れる | WinForms でリーク/不定期障害 | 「寿命が長い箇所で参照が残る。破棄と解除が揃うと再現性が上がる」 |
禁書庫A: 逆引き(症状→原因→対策)
| 症状 | ありがちな原因 | 切り分け(見る場所) | 最短の対処 | 再発防止(ルール化) |
|---|---|---|---|---|
| レビューが名前論争になる | 命名規則が曖昧、対称語が揺れる | 命名規則、用語表 | 用語表で揺れを固定 | 命名/対称語を規約化 |
| 障害の原因が追えない | 例外が消える、ログが足りない | catch 範囲、ログ出力 | ログ追加+throw; | 例外/ログの扱いを規約化 |
| 変更が怖くて触れない | public が広い、依存が増殖 | アクセス修飾子、static | スコープを絞る | 公開範囲と依存方向を規約化 |
| WinForms が重い/落ちる | event/Timer/Dispose 漏れ | 登録解除(イベントを外す)、IDisposable | using/解除/破棄 | 寿命管理を規約化 |
禁書庫B: 早見表(確認ポイント一覧)
- 命名: ファイル/型/namespace の一致、対称語の固定、bool は true の意味で読む
- 例外: 対処できる場合のみ catch、再 throw は throw;、例外でフロー制御しない
- ログ: 何を残せば原因追跡できるか(入力/識別子/例外)を揃える
- スコープ: public を増やさない、可変 static を避ける、変更可能コレクションをそのまま公開しない
- 寿命: IDisposable は using/Dispose、イベント登録解除(イベントを外す)、長寿命が短寿命を捕捉しない