コーディングの指針
弊社プログラムの開発をする人の向けにこのようなプログラムを書いて欲しいというのを書き出しました。内容は賛否あると思いますが、自分がコードを書くときやレビューするときに考えていることをまとめています。
内容は運用してみてから修正していきます。雑多に書いたので読みにくいですがよろしくお願いします。
はじめに
よくプログラムを作る速さが評価されてしまうことが多いが、変更を積み立てるアジャイル開発においては、作成する速さよりも更新や機能追加の速さがそのまま開発速度につながるため、メンテナンス性の高いプログラムを書くことはプログラムを作ることよりも重要となる。
プログラムは更新やメンテナンスなどのために何度も読む必要が生じる。書く回数よりも読まれる回数の方が多く、読みやすく動作を把握しやすいプログラム書くことはメンテナンス性の向上に役立つため重要である。
上記の目的を達成するためにコーディングの指針を定義する。指針については下記の考えを中心に取り決めている。
- ロジックを明確で短いものにする
- 把握すべき状態を少なくする
- 他人が見て分かりやすいものにする
分かりやすさについては主観的なものも含まれるが、言語自体やライブラリ、メンバー、プロジェクトの常識とその状況に照らし合わせて最適なものを話し合いにより決定する。
基本事項
言語で標準的なコーディング規約が定められている場合は各言語のコーディング規約に基づく様にする。下記で定める指針が言語のコーディング規約と衝突する場合は理由がなければ言語のコーディング規約を優先とする。各言語のコーディング規約は標準ライブラリで使用されているものを推奨とするが、他のものを使用する場合プロジェクトの初期で定義すること。（ただし、C#のインターフェース名の前に「I」のプレフィックスをつけることや定数を全て大文字にする必要はあまりないので推奨しない。）
リファクタリングなどを通じて、コードが下記指針に沿うように定期的に編集を行う。コードを修正している途中で規約に当てはまらないものを見つけたら修正を行うこと。その際にコミットを分けることが望ましいが、後にすると忘れ去られることが多いので1ファイル数行程度であれば、関係のないコミットに含めて良いこととする。
プログラムで必要のなくなった部分は躊躇なく削除すること。また、うまく設計できていないものを書き直すことを恐れないこと。良くないコードにぶら下がってシステムを作るコストは、コード自体を書き直すコストよりも高くつくと考える。
バージョン管理について
コミットは意味のある単位で行うと良いが、masterブランチ以外の作業ブランチではなるべく頻繁にコミットとプッシュを行うことを推奨する。これだと他人から見てあまり意味の無いコミットが履歴に表示されてしまう。githubプルリクエストでmasterに統合するときは、「Squash and merge」を使用することで意味ある履歴が残るようにする。「Squash and merge」は状況により使用しにくい時があるので使用できないときはその他のマージ方法を使用する。
branch名は何をやっているのか分かりやすいものをつける。作業者の名前などをつける必要はない。
識別子の推奨事項
識別子の命名は可読性に大きな影響を与えるため非常に重要な仕事である。（ここではルールだけ定義するが、状況によって適切な名前を定義できるかどうかはかなりの技量を要する技術だと考える。）
識別子の英単語の使用、略語の禁止
担当者間でブレの防止のため、プログラムの変数等識別子として英単語を使用し、スペルチェックプログラムを導入し、問題とならないものを使用する。
ローマ字は対応する英単語がない場合は以外は非推奨とする。また、英単語の省略も一般的でないものは使用禁止とする。「Hyper Text Transport Protocol」を「HTTP」とするなどは一般的であるが、statusという単語を「sts」など文字を切り詰めて省略することは可読性に悪影響を及ぼすためやブレを生じるため禁止とする。だたし、administratorなどの特に長い単語は「admin」とすることも多く、これらについては推奨はしないが使用する場合は、プロジェクト内で単語一覧として管理する。(C#などは省略しないでやたら長い関数名も多い。)
社内で作成されたシステム名や機能名などの語句については、単語一覧として管理し、使用することができる。
単語一覧についてはスペルチェックプログラムの入力になるため、スペルチェックプログラムの設定ファイルを単語一覧としても良い。
識別子の付け方
識別子の付け方は、各言語のルールに則るが無い場合は下記を推奨する。
- 関数は動詞で始める。
- リストなどのコンテナデータ構造は英語の複数形にするか、listなどをつけてわかるようにする。
また、関数などで識別子にない処理を行わない様にする。例えば「GetItem」関数でデータを取得するだけでなく更新をしている場合は、「GetItemAndUpdateAccessTime」の様にする。
スコープと識別子
変数は使用する場面を絞ることが望ましいため、変数はスコープが最小限となるようにする。ローカル変数でよいものをオブジェクトのフィールドとして定義しないことや、privateで良いものをpublicで定義しないようにする。ローカル変数でも関数が長い場合などはスコープが小さくなるようにする。
変数名はそれが使用される文脈を考慮に入れてどのような識別子がよいかを客観的に考える。スコープが大きくなると使用される状況も広くなるのでより広い視点で考慮を行うようにする。private変数であれば、そのクラスやファイル内で通じるような名前で良いが、public変数である場合、それを使用するパッケージ全体や外部のコードからも使用される場合は、外部の使用者が見てわかるように客観性をもって命名を行う。
関数の引数の名前は外部から見えるものなので、きちんと考慮するようにする。
プレフィックスについて
識別子の前にプレフィクスを「ADN_」をつけるなどはオブジェクト指向言語ではクラスやネームスペースへの分割を適切に行えば不必要で、可読性にも悪影響を及ぼすため、基本的には使用しない。
データの型を表すハンガリアン記法などの方式も可読性に悪影響を及ぼすので基本的には使用しないが、ローカル変数で同じデータの型が複数ある場合など、型を明示することは良いが、わかりやすく明示するようにする。
コーディングの推奨事項
フォーマットツールの使用
作成されたコードは必ずプロジェクト標準のフォーマットツールでフォーマットを行うこと。
ただし、外部から受け渡されたプログラムで外部と協業で作業を進めている場合で、フォーマットを行うと協業がしにくい場合は、フォーマットは協業が終了するまで遅らせてもよい。
一行の最大文字数
一行の最大文字数は80〜120文字とする。80文字を推奨とする。
行数の長い関数の禁止
長い関数は処理が追いにくくメンテナンスに支障をきたす。また、再利用性の低さや変更に対し脆弱で品質的に劣ることが多い。関数の長さは通常のディスプレイで一画面に収まる量、およそ50行程度を限度とし、それ以上長くなる場合は、意味をもつ単位で他の関数に分割することが望ましい。
ただし、設定ファイルをわざわざ作るほどではなく、データやクラスの構成などで同じ文を繰り返す場合などは処理を追う必要はないのでこの限りではない。このような場合にif文がないことを条件に長い関数を書いてもよい。
行数の長いファイルの禁止
行数の長いファイルもメンテナンスに支障をきたす。一つのファイルには一つの完結する役割をもたすべきで。1ファイルの行数は500行程度を限度とし、それ以上大きくなる場合は、役割を固有の役割を持つファイルにコードを分割し、個々のコードが協業して、一つの役割をこなす様にする。
なお、基本的に1クラスは1ファイルとするので、クラスも分割することになる。
コメント
コメントは有用であるが、コメントを多用するよりもコメントが必要無いぐらい読みやすいコードを書くことが重要で、コメントの記述は処理の解りに辛い部分に絞るべきである。理由は下記でコメントが多くなると負荷がかかる。
- コードを修正した際にコメントも修正しなければならない。
- コードに対しコメントの内容が正しいことを保証できない。
ただし、JavaDocなどのドキュメント用のコメントは使用者にコードの使用方法を示すために重要なので推奨するが、入力データ型と何をするかが関数定義から分かるようになっていれば必ずしも必要はない。
また、語尾は最後まで書く。「〇〇〇の確認」ではなく「〇〇〇の確認をする」など。特に「〇〇〇処理」などのコメントは曖昧になりやすいので避ける。
デバッグ
エラーのないコードは存在しないと思うこと。リリース前にテストは行うが、開発者はそれだけに頼らずに、常にコードを実行させながら開発を進めること。
コードは実行すれば実行するほど洗礼されエラーが少なくなる。そのために常にコードを実行できるように環境を整備すること。一度も実行していないコードを絶対にリリースしてはならない。
コード記述の推奨事項
乱用を行わない
言語の機能やライブラリには本来想定される用法というのがある。本来想定される用法を超えて乱用を行うことは避ける。基本的には公式のドキュメントに精通していればそのようなことは避けられる。
コードコピーの禁止
システムのメンテナンスはロジックの量が少ないほど安易になる。そのため、同じ様なコードを書かずに似たロジックは一つにまとめる様にする。また、コードを共有化するためのテクニックを使用する。大きなコードの使い回しは複雑になることがあるので、元のコードを小さくすることを検討する。
理由なくコードを別の場所にコピーしてそのまま使用することは絶対に禁止する。
ロジックをシンプルに保つ
ソースコードは複雑な書き方をせずに、明確で分かりやすい書き方をすること。一行に二つ以上の処理を入れたり、二つの処理を織り交ぜた様なコードを書かないこと。
言語の新しい機能でシンプルに分かりやすく記述できるようであれば、そちらを積極的に使用すること。例として下記を挙げる。
「for (let i = 0; i < list.length ; i++) { print(list[i]); }」よりも「for (const obj of list) { print(obj); }」の方が望ましい。「i」に対する操作はいくつか考えられるため、処理を理解するために解読する必要が生じる。リストを繰り返すという目的から考えると「i」に対する操作がない方が処理の目的を明確化できる。
配列の操作は使えるのであれば、高階関数を使用したものにする。（JavaScriptでいうmap, filter, reduce)
// 非推奨
const result = [];
for (const value of list) {
result.push(value * 2);
}
よりも、 下記の方が目的を明確化できる。慣れるとlistの各項目に2をかけると読める様になる。
// 推奨
const result = list.map((v) => v * 2);
Promiseのthenやcatchを使用するよりもawaitとtry...catchを使用した方が望ましい。try...catchは言語の基本機能だが、PromiseのthenやcatchはPromiseの独自の機能な上コーディングが制限される。
どのようなデータが入力されるのかを明示する
型定義のない言語で複雑な型を受け付ける関数や複数の型を受け付ける関数を作成した場合、作成者以外がどのようなデータがくるのか、どのようなパターンが存在するのか分からなくなってしまうことがある。
基本的に型の定義ができる言語を選ぶが、そうではない場合入力されるデータの形式を明示すること。明示方法としてはコメントでの記述や単体テストプログラムなどを使用する。また、複数の型を受け付け、動作が変わるようなプログラムは作成しない方が望ましい。(ここではオブジェクトとnullで動作が変わるといったことも含む。)
また、せっかく型定義のある言語を使用しても、なんでも文字列型を使用するなどを行うと、入力データが不明瞭になるため、そのようなことは控える。入力型は制限の多いものを優先的に使用する。
動的型付けの言語など作成時はとにかく動かして色々なパターンで試行錯誤することも多いが、そのため作成者しかわからないプログラムができてしまう可能性が高くなる。assertなどで入力データの定義を行うことも有効である。
イミュータブル
言語によってサポート状況は異なるが、変数はなるべく書き換えないように制限を設けておいた方がシンプルに構成でき、状態を把握しやすくなる。C#であればフィールドにreadonlyを付けておけば、コンストラクターで書き込んだ後は、書き換えられなくなる。コンストラクター実行中はデータが他と共有されることはないため、変数の値を書き込むには絶好の場所であることを意識する。
状態を少なくするようにする
処理自体や時間によってたくさんの状態をもつプログラムについては、すべての状態やその組み合わせを把握するのは難しくバグを生み出しやすい。なるべく状態を少なくすべきで設計やコーディングにおいて下記のようにすることを推奨する。
- 構造化プログラミングで言うと順次と特に選択は少なくすべき。if文に関しては、Strategyパターンなどを使用し、オブジェクトとオブジェクトの関連性を重視するように設計するとよい。クラス設計が重要になる。
- 変数やメンバーフィールドにおいてはイミュータブルを多用すべき。
- サービスに関してはステートフルよりもステートレスで設計を行う。
また、決められた手順で実行されるべきオブジェクトメソッドなどは内部に状態を持っているので避けるべきである。
複雑な状態遷移をするものは遷移図やシーケンス図などの資料を作成すること。
エラー対処の推奨事項
例外の対処
try...catchが階層的になりすぎるとプログラムの見通しを悪くする。不必要なthrowとcatchを作らない。スローするのは例外がここではリカバリ不可能で、キャッチするのは、リカバリ可能か、リカバリをあきらめて他のシステム化ユーザーに通知するときにする。ただし、ライブラリの使用者などのために元々のエラーに現在の状況を付与して再スローする場合はキャッチしても良い。再スローするときは基本的にアプリケーション独自に作成した例外型で投げる。
想定外のエラーに必要のない対処しない
ロジックを複雑にするだけでなく、エラーを隠すことになるため、想定外のエラーに無理に対処してはいけない。想定外のことが起こったら、すぐにエラーの表示を行い、使用者から分かるようにする。エラーが素通りしてしまってリリースされてしまう危険を避けることができる。
また、try..catchなどの例外機構では想定していない例外(Exception等)を拾わない。想定外のエラーはトップレベルで拾ってユーザーに通知する。また、アプリケーションからExceptionなどのどこでも発生する例外を投げると、呼び出し元のコードで想定していない例外をキャッチすることになってしまうため、基本的にアプリケーション独自に作成した例外型で投げる。標準ライブラリに合致する例外があればそれを使用しても良いが、その例外が投げられた時の状況が外から見て一意になるようにする。
設計の推奨事項
SOLIDの原則
コードはSOLIDの原則に沿う様にする。
- 単一責任の原則 (single-responsibility principle)
- 開放閉鎖の原則（open/closed principle）
- リスコフの置換原則（Liskov substitution principle）
- インターフェース分離の原則 (interface segregation principle)
- 依存性逆転の原則（dependency inversion principle）
複数の機能を持つクラスを作らない
ある一連の機能を作るときにそのまま一つのクラスを作成せずに、どういった機能を持つオブジェクトとオブジェクトがあればそれを実現できるか考える。各オブジェクトは単一責任の原則に則り、オブジェクトとオブジェクトが相互に作用して機能を実現できるようにする。
デザインパターン
使用は自由であるが、下記について特記する。
- Singleton: staticよりも良いが安易に乱用することが多いので基本的に使用を控える方向で考える。Singletonは同じクラスを同時に複数使用する場合やプログラム中で切り替える場合などに破綻し、プログラムを大きく書き換える必要に迫られることがある。また、初期化時にパラメーターを入れる場合、初期化されたことを使用者側で保証するのが難しい。Singleton同士が関係もつようなプログラムも難しい。
- Strategy: プログラムの一部を共通化するときに使用を推奨する。Abstract Factory, Factory Methodパターンと組み合わせて使用する。複雑化すると何をどこで処理しているのか分かりにくくなるTemplate Methodパターンよりもこちらを推奨する。実装に色々な方式が考えられる場合、Strategyとして実装するとインターフェースとして明確化するし、後から切り替えることも可能になる。
- Facade: 複雑なクラスを分割する際にFacadeを使用して分割する。インターフェース分離の原則を意識する。