Kokudoriing

技術系与太話ブログ

Coding Guidelines for C# 3.0, 4.0 and 5.0を読んで

Dennis Doomenさんとやら(よく知らない)がお作りになった「Coding Guidelines for C# 3.0, 4.0 and 5.0」
というC#のコーディングガイドラインがあるんですが、それが日本語翻訳されたらしいとのこと。

Coding Guidelines for C# 3.0, C# 4.0 and C# 5.0 - Download: Coding Guidelines for CSharp 3.0, 4.0 and 5.0

早速読んでみたのでその感想を。
当たり前ですが万人受けするガイドラインなんてあるはず無いのでいくつかは僕には合いませんでした。のでその理由も。
そして間違ってたらこっそりゆっくりまったり優しくご指摘いただければ幸いです。

・The Principla of Least Surprise(最小の驚きの原則) は、ソリューションには人々が理解できないものや、
間違った方向に入るものを含むべきではないことを意味する。

いきなり重箱の隅つつきですが、驚き最小の法則は「利用者の期待に合わせるべきだ」という趣旨であって理解はともかく間違ってるかどうかは問いません。(少なくとも人間工学の文脈の中では)
よくあることですが、正しい方法のほうが何かと理解されません。
もちろんこれは技術的負債とのトレードオフです。

ReSharper は、インテリジェントなコード検証エンジンであり、
いくつかの構成を行うことでコーディングガイドラインの多くの側面をすでにサポートしている。

ReSharperに言及してるコーディングガイドラインってだけで素敵ですね。
まぁもちろんVisual Studioに依存しちゃってる(つまりWindowsに依存しちゃってる)訳ですが・・。

あなたの決定を助けるために、それぞれのガイドラインに重要度を割り当てた:
壱 あなたがスキップしてはいけない、すべての状況で適用すべきガイドライン
弐 強くお薦めするガイドライン
参 すべての状況で適用する必要はないが、推奨するガイドライン

壱があるのは言語設計上のバグとしか言い様がない(もちろん設計上の原則なら仕方ないですけども)。
バーランドメイヤー先生がOOSC本でも仰ってましたが、絶対にすべきでないのなら言語がそれを許すべきじゃないです。
まあ、C#で壱はいくつか思い当たるわけですが・・。

オブジェクト指向入門 第2版 原則・コンセプト (IT Architect’Archive クラシックモダン・コンピューティング)

オブジェクト指向入門 第2版 原則・コンセプト (IT Architect’Archive クラシックモダン・コンピューティング)

( AV1001 参)
オブジェクトが使用可能になるまでに追加でプロパティに値をセットする必要がないように設計する。

OOSC本ではコンストラクタはオブジェクトのクラス不変条件を成立されるための機構だと説明されてましたし僕も同意します。
オプショナルな値セットなら良いんですが、コンストラクタを通したのにそのオブジェクトが使用不可状態(InvalidOperationExceptionをスロー)
というのは罠過ぎますし誰も得しないです。
コンストラクタのパラメータの数が増えるのを懸念してるっぽいですが、
パラメータを別オブジェクトにまとめるなりファクトリにするなりあるんじゃないですかね。
個人的にこれは壱、最低でも弐かなあと。
「すべての状況で適用する必要はない」のであれば「どのような状況であれば適用にしなくても良い」のかの理由がほしいなあ。

( AV1135 壱)
文字列やコレクションのプロパティ、メソッド、引数はnullであってはならない。

うーん、.NETにnull非許容参照型が無いのが大変悔やまれる。
勿論これは大賛成なんだけども、じゃあ現実問題どうしよう。null入れんなってルールでnullチェックは一切しない?
簡単な線引ならpublic/protectedメソッドはnullチェックしてNullArgumentException、
privateメソッドはノーチェックあるいはnullチェックアサーションあたりが無難?
あとプロパティでNullArgumentExceptionって出して良いもんなんですかね?というかプロパティで例外全般を出して良いのかしら。
なんとなくプロパティは軽量アクセサに留めて例外投げない(NullArgumentExceptionも)方が自然な気もするが果たして。
ちなみにnullチェック以外には防御的プログラミングという、
nullが来たらデフォ値に変換したりなんやかんやして出来るだけ処理できるように頑張るという方法もある。
個人的にはnullチェックして弾くほうが好みだけど、どちらが好ましいんだろう?

( AV1140 参)
プリミティブよりもドメイン固有の値型を検討する。

マーティン・ファクラーさんのPoEAAで言う所のバリューオブジェクトですね。
これには賛成なんですが、C#(というか.NET)は結構サポートひどいですね。
まずバリューオブジェクトは原則イミュータブルであるべきです。(String型のように破壊的メソッドを定義しない)
次に、値等価性をサポートすべきです。が、これが結構面倒です。
まずEqualメソッドをオーバーライドします。次に==演算子もオーバーライドするでしょう。
さて、==演算子が等しいオブジェクト同士はGetHashCodeメソッドの戻り値も等しくなければならないというルールがあります。
つまり、==演算子をオーバーライドしたのならば、GetHashCodeメソッドもオーバーライドしなければなりません。
またGetHashCodeはインスタンス毎に固有の異なるハッシュ値を返さなければなりません。
また、ハッシュ関数なわけですからランダム分布するように工夫する必要もあります。
うーん、コスト高いなあ・・。
因みにPoEAAで「.NETはバリューオブジェクト用に構造体って機構があるから便利だね」的なことが書かれてますが、
.NETの構造体は意味的でなくメモリ配置的な分け方であり、構造体の積極的な使用は推奨されてません。
クラスだろうとイミュータブルなら振る舞いは同じです。
参考までに、プログラミング .NET Framework 第3版によると、構造体はおよそ16バイト以下のデータを格納する際に考慮すべきだそうです。

エンタープライズ アプリケーションアーキテクチャパターン (Object Oriented Selection)

エンタープライズ アプリケーションアーキテクチャパターン (Object Oriented Selection)


プログラミング .NET Framework  第3版 (マイクロソフト公式解説書)

プログラミング .NET Framework 第3版 (マイクロソフト公式解説書)

( AMV1202 弐 )
リッチで意味のある例外メッセージテキストを提供する

プログラミング .NET Framework 第3版によると、
例外メッセージはエンドユーザーに見せない事を前提とし、技術者向けの内容にすべきだそうです。

( AV1130 弐 )
コレクションクラスの代わりにIEnumerableやICollectionを返す。
( AV1250 壱 )
LINQ式を戻り値として返す前に評価する。

んー、これはどうなのか。即値を返すのにIEnumerableを返すのはめちゃくちゃややこしいので是非ともやめて頂きたい。
というか後者が壱なのは何なのか。メソッドチェーン全否定というかLINQ to Object全否定というか・・。
素直に継続を意味するのならIEnumerableを、値を意味するのならListなりなんなりの具象型を返せばいいのでは?

( AV1500 壱 )
メソッドは7ステートメントを超えるべきではない。

複雑性を数値化する動きは総じて嫌い。
7の根拠は何?7±2とかいう有名なマジックナンバー?それとも経験則?
ステップ数でなく責務の数とかで考えたほうが建設的じゃないですか?

( AV1501 壱 )
デフォルトでは、すべてのメンバーをprivateに、型をinternalにする。

C#ではメンバの可視性のデフォルトはprivate、型はinternalです。つまりデフォのままで行けと。
ここまで書くのなら「デフォルトの修飾子と同じ時は修飾子を省略すべきかどうか?」にまで踏み込んでいただきたかった。
因みに自分は省略する派です。アンダース・ヘルスバーグさんがアクセス修飾子のデフォルトを決めた気持ちを汲み取りたい的な意味で。

( AV1502 弐 )
二重否定の条件を避ける。
customer.HasNoOrdersのようなプロパティは間違っていないが、以下のように否定条件で使用するのは避ける。
bool hasOrders = !customer.HasNoOrders;

これはHasNoXxx的な否定的プロパティ/メソッドを定義すること自体は容認していると言う事?
ただただ罠っぽいからHasXxx系に統一したいんだけどもなあ・・。

( AV1515 壱 )
“マジック” ナンバーを使ってはいけない

まあ王道ですね。勿論異論ないです。
ただ、ガイドライン上ではサンプルコードとしてconstが使用されてます。
原則としてconstの使用は控えましょう。代わりにreadonlyをどうぞ。
constは完全なマクロです。つまりコンパイル時にコードレベルで値を展開します。
なので別アセンブリにまたがるconst値がある場合、
const値が何らかの理由で変更しなければならない場合、最悪再コンパイルが必要です。
readonlyはイミュータブル(最大入不可)が保証されるだけのただの変数なのでそのような問題は発生しません。
constで得られるメリットよりもconstで被るデメリットの方が多いケースが殆んどです。
constを使用するならば、値は絶対に変更しないことを保証しなければなりません。

( AV1520 壱 )
型が確実に明確なときにだけvarを使用する。

何故皆さんそんなに型推論を怖がるのです?しかも壱・・。
よく「int, float等の基本型はvar無しで」と言った事を聞きますが、メリットは何です?
int num = 3; が var num = 3; になると生産性が落ちるのです?
また、ガイドラインに以下のようなサンプルコードが。

var myfoo = MyFactoryMethod.Create("arg"); // なにを期待しているのかが明確じゃない。

何故戻り値の型を知らなければならないのでしょう?
そもそもオブジェクト指向というものを考えた時、オブジェクトとは対象の振る舞いによって定義されます。
歩いてほしい。グワグワと鳴いてほしい。であるならばガチョウだ。という風に。
ガチョウだから歩いてほしい訳ではありません。
オブジェクト指向とはドメインのモデル化であって現実のモデル化ではありません。
必要のない振る舞いなど、いくら正しかろうが必要ありません。
我々が欲しいのはその型が何であるかではなくそれが何を出来るか、です。
そんなものはIntelliSenseが教えてくれます。
人間様がわざわざ型なんてものを意識しなければならないだなんて息苦しいだけです。
あと、前述のサンプルコードの場合問題はMyFactoryMethod.Createという名前が酷すぎる点であり、
期待が明白な名前であれば戻り値の型などわざわざ明記する理由はありません。
int, float等の基本型に関してはリテラル表記によって型が変わりますが、
これもリテラル表記の表現力が増えただけであり特に問題はないように思えます。
(リテラル表記を知らないC#erが要るのなら、基本型に限り型を明記してもいいかもしれませんが、
自分ならリテラル表記を勉強するように言います。)

( AV1525 壱 )
true やfalse を明示的に比較しない。

while (condition == true) のようなtrueとの比較は全く意味が無い。
これは while (condition) と全く等価になる。
C#はifやらwhileやらの条件式はbool値しか書けないので紛らわしさも発生しない。
ただ、while (condition == false) の場合は少し複雑になる。
これを推奨する形で書くと、while (!condition) となる。
いくらなんでも ! は視認性が悪すぎる。一文字違うだけで意味が正反対になるだなんて怖すぎる。
なので自分は ! でなく == false を使ってる。
CoffeeScriptやらRubyやらみたく not 演算子が欲しいですね。

( AV1536 壱 )
常にswitch ステートメントの最後のcase の後にdefaultブロックを追加する。

以下の様なサンプルコードが掲載されていた。

void Foo(string answer)
{
  switch (answer)
  {
    case "no":
      Console.WriteLine("You answered with No");
      break;
    case "yes":
      Console.WriteLine("You answered with Yes");
      break;
    default:
      // ここで終わることはない。
      throw new InvalidOperationException("Unexpected answer " + answer);
  }
}

この場合は勿論これでOK。
ただ、この時のanswerが列挙型(Enum)なら、defaultではInvalidEnumArgumentExceptionあたりを投げるべき。
これは.NETクラスライブラリ設計に書いてました。確かにEnumってキャスト出来ますからねー。

.NETのクラスライブラリ設計 開発チーム直伝の設計原則、コーディング標準、パターン (Microsoft.net Development Series)

.NETのクラスライブラリ設計 開発チーム直伝の設計原則、コーディング標準、パターン (Microsoft.net Development Series)

( AV1540 弐 )
複数のreturn ステートメントを控える

これは全く同意できない。ガード説としてのreturnが無いと煩雑になるケースが(経験的に)多い。
returnを1つにしてフローさせた時の可読性ってそんなに高いかなあ・・。

( AV 1562 壱 )
ref やout パラメータを使用しない。
代わりに複合オブジェクトを返すようにする。

.NETのタプル(Tuple)がイケてればTuple使うべきパターンかもでしょうが、まぁお察し。
しかし戻り値のためにわざわざ型作るってのはなんとも芋っぽいですね。しょーがないけども。

( AV 1575 壱 )
コードをコメントアウトしない。

トラッキングシステム使ってそっちで書けとのこと。
まぁ普通にボブおじさんがClean Codeで言ってるみたくバージョン管理システムのコミットコメントに書くのが良いと思います。
因みに自分はコメントが大嫌いなのでドキュメンテーションコメントとHACK的なコードの説明コメントしか書きません。

Clean Code アジャイルソフトウェア達人の技

Clean Code アジャイルソフトウェア達人の技

( AV1701 壱 )
アメリカ英語を使用する。

あと出来ればベーシック・イングリッシュから使用していただきたく・・。
(難しい英語読めないのです・・、ごめんなさい。)
ただ、「CanScrollHorizontallyというプロパティ名はScrollableX よりも優れている」はどうかと。
もうX,YなんてInput/OutputをIOと言うくらいには普遍的じゃない?
わざわざInput/Outputなんて書かないんだしX,Yくらい別に良くない?とか思ったり。

( AV2305 弐 )
すべてのpublic、protected、internalタイプとメンバーにドキュメントを記述する。

えー、privateメンバも書こうよ。
自明なのは良いですけども、できるだけ書くべき。折角良いIDE使えてるんだから最大限活かすべき。

( AV2400 壱 )
4文字の空白インデントを使用して、タブを使用しない。

うぐ・・、TAB派は辛いなあ・・・。
まぁ、空白の方が良いよね。うん・・・・。

( AV2406 壱 )
メンバーを適切に定義された並び順にする。

ないない、ありえない。
なんでprivateフィールドとpublicプロパティを話して書かなくちゃいけないのよ。
しかもC#にはregionディレクティブがあるんだから論理的な凝集性高めるべき。
ただそれらと独立したコンストラクタやら何やらを大体上の方に書くとかいうルールは有用だと思う。
ただざっくり、private field→public property→constructor→ext... くらいで良いんじゃないですかね。

( AV2407 壱 )
#regionを控える。

おうふ・・言ったそばから。
「Regionは便利なときもあるが、クラスの主な目的を隠すこともできてしまう。」
んー?主な目的とは?
確かにregionは色々な物を隠しますが、実装を隠して抽象化するのはカプセル化の原則では?


というわけで以上です。
振り返ってみると愚痴ばっかり言ってる気もしますが非常に良いガイドラインなんじゃないでしょーか。
言葉が若干崩れてるのは眠いからです。
おやすみなさい。

Command/Query分離原則とreturn this

Command/Query分離原則はバーランド・メイヤー大先生のありがたいお言葉です。
内容はと言うと、

オブジェクトやシステムのインターフェイス&実装を「CommandとQueryに分けろよ」という提案。
Commandは値を持たず、副作用(つうか、主作用だけど)だけを持ちます。
Queryは副作用を持たず値だけを返します。

「メイヤー先生の偉大さとCommand-Query分離 / 檜山正幸のキマイラ飼育記」
http://d.hatena.ne.jp/m-hiyama/20101216/1292469108 より引用

以下はCommand/Query分離原則を順守した簡単な例です。

var stream = new Stream();

stream.write(item); // command

var result = stream.read(); // query

なるほど、CommandとQueryが分離されているので責務の境界が明白です。
書くと言えば書くんです。読むといえば読むんです。非常にわかりやすい。

・・ですが、ついついこういう風に書きたくなってしまいます。

var stream = new Stream();

// writeはvoidでなくthis(この場合はstream)を返す
var result = stream.write(item).read();

CommandとQueryを分離する事の意義は非常によくわかります。
しかし、何故Commandはvoidを返す(つまり値を返さない)のでしょう?

Martin Fowler卿の仰るところによると、CommandとQueryの分離を明示的に示すためです。

この原則は、戻り値の型によって両者を区別するというものである。
たいていの場合、うまく機能するため、これはよいルールである。

「CommandQuerySeparation / Martin Fowler's Bliki in Japanese」
http://capsctrl.que.jp/kdmsnr/wiki/bliki/?CommandQuerySeparation より引用


ただ、両者を区別したいのであれば他に良い方法もあったのではないでしょうか。
例えばJavaにおけるgetterや、C#におけるgetプロパティはQueryそのものです。

であればsetter(Command)がvoidを返さなければならない理由は何でしょうか?

考えられる理由としてはreturn thisをする意味的理由が無いため、です。
それの何が問題になるのか、例えばCommandが値を返すケースを考えます。

ん、上記では既にCommandは値を返さないと説明をしたわけですが。
これはファウラーやメイヤー自身も認めていることですが、Command/Query分離原則にはいくつかの例外があります。

Meyerは「コマンド・問い合わせ」原則を全面的に使用したいと考えているようだが、 そこには例外がある。
スタックのpopが、状態を変更するモディファイアの良い例だ。
Meyerは、「このメソッドはできるだけ避けたほうがよいが、便利なイディオムである」と明確に述べている。
なので、私はできるだけこの原則に従うようにはするが、原則を破ることもいとわない心構えでいる。

「CommandQuerySeparation / Martin Fowler's Bliki in Japanese」
http://capsctrl.que.jp/kdmsnr/wiki/bliki/?CommandQuerySeparation より引用

確かにStackにおけるpopメソッドは副作用(Command)と値の取得(Query)という2つの顔を同時に持ちます。
この時、Stackのpushメソッドの戻り値がvoidであればCommandであることが一目瞭然です。
しかし、pushがthis、すなわちStackを返すのであればどうでしょう?


が、そもそもpush/popなどは名前(もっと言うとシグニチャ)から用意にCommandであるかQueryであるかが把握できる。
というか利用者側にCommandかQueryかを把握できるような名前を付けるべきであって、
戻り値でしか判別出来ないのであれば設計ミスなのでは?と。

そしてCommandがreturn thisしてくれるならば先述したように、どうせ一行後に書くCommandをQueryと同じ行に書けるようになる。
そのベネフィットの大きさはともかくとして、メリットは有るわけだ。

例えばjQueryはCommandであるはずのjQuery.fn.eachはreturn thisしている。
(C# Ix のIEnumerable#ForEach拡張メソッドはvoidを返している)

jQueryやLINQの影響もあってか、かなり前からメソッドチェーン記法が世間的に受け入れられているような気がする。
どうだろう。戻り値でCommandかQueryかを判別する旨味はそこまであるのだろうか。


蛇足.)

言語がこの考えをサポートしてくれればいいのに、と思ったりする。
言語が状態を変更するメソッドを検知したり、 プログラマが目印をつけたりできる、というのはどうだろう。
言語が自動的に検知できないのは、状態を変更しないというルールがシステムのObservableStateにしか適用できないからである。

「CommandQuerySeparation / Martin Fowler's Bliki in Japanese」
http://capsctrl.que.jp/kdmsnr/wiki/bliki/?CommandQuerySeparation より引用

状態を変更、までは行かないまでも、C#のプロパティはCommand/Queryを言語レベルでサポートできているような気もする。
問題は非同期であって、これはどうだろう・・。
C#5.0にはasync/await構文が入ったので非同期を同期っぽく書けるにはかけるが、asyncはプロパティには非対応だし・・。
やるとしたらRxで、とかなんですかね・・?

ただ命名規則でgetter/setterを区別するなら、Queryはgetプレフィックス付けておけば解決する問題なような気もする。
あと、副作用を隔離する事に長けているHaskellが予測可能性高いのもこういった理由なのかなあと。