StyleCopで学ぶC#コードスタイル入門

手なりでコーディングしているとコードのスタイルがハチャメチャになりがちです。規約に従って書こうと思っても規約に従ってかけてるかを判断するのは意外と難しいものです。そんならツールを使ってチェックすれば抜けもなくて良いじゃん?ということでMS謹製のC#静的解析ツールStyleCopを使ってみることにします。

StyleCopのチェック内容が意味不明とか思ったら以下のサイトが参考になります。

ちなみに、もっとC#らしいコーディングスタイルを知りたいと思う方は以下の記事が参考になると思います。

StyleCopのインストールと設定

以下のページからStyleCopをダウンロードしてインストールします。

VisualStudioを開いてWPFアプリケーションプロジェクトを作成します。csコード上でコンテキストメニューを開くと「Run StyleCop」という項目が追加されているはずです。

早速実行してみると…

プロジェクト作成直後のなのにいっぱい警告が出てしまいます。警告内容の大半が英語を前提としてコメントについてなのでStyleCopの設定でチェックをしないようにしてしまいましょう。ソリューションエクスプローラのプロジェクトのコンテキストメニューを「StylCop Setting」選択して「Documentaition Rules」のチェックをはずしましょう。


この設定後StyleCopを実行しても13個の警告が出ています。

usingはネームスペースの中におく

警告メッセージを確認すると

SA1200: All using directives must be placed inside of the namespace.

となっています。これは、

using System;
using System.Collections.Generic;
// using 省略

namespace WpfApplication1
{
}

namespace WpfApplication1
{
    using System;
    using System.Collections.Generic;
    // using 省略
}

と書き直せば消えます。個人的にはこの書き方を好きになれないのですがどうなんでしょう?Javaに近くした感じなのでしょうか?
ともあれ、これで警告は消えたはずです。では、コードを追加してみましょう。とりあえずボタンを作ってClickイベントにイベントハンドラを登録してみます。

public MainWindow()
{
    InitializeComponent();

    var button = new Button();
    button.Click += new RoutedEventHandler(button_Click);
}

void button_Click(object sender, RoutedEventArgs e)
{
    throw new NotImplementedException();
}

この状態でStyleCopを実行すると警告が3つ出ます。順番に見ていきましょう。

アクセス修飾子は必ず記述する

SA1400: The method must have an access modifier.

最初はこれですね。これはメソッドにpublicやprivateなどのアクセス修飾子をつけば消すことができます。

// privateを追加
private void button_Click(object sender, RoutedEventArgs e)
{
    throw new NotImplementedException();
}

メソッド名はPascal記法を使う

SA1300: method names begin with an upper-case letter: button_Click.

こいつはメソッド名の頭文字を大文字にしてあげれば消えます。VisualStudioでこの辺自動でやってくれないかなぁとかよく思います。

// Pascal記法に書き直す
private void Button_Click(object sender, RoutedEventArgs e)
{
    throw new NotImplementedException();
}

自クラスメンバのアクセスにはthisを使う

SA1101: The call to Button_Click must begin with the 'this.' prefix to indicate that the item is a member of the class.

自クラスのメンバにアクセスする場合はメンバコールの前に「this.」つけろということですね。

public MainWindow()
{
    InitializeComponent();

    var button = new Button();

    // 「this.」つけてメソッドコール
    button.Click += new RoutedEventHandler(this.Button_Click);
}

これで全ての警告が消えたはずなのでさらにコードを追加してみます。

private int count;

private void Button_Click(object sender, RoutedEventArgs e)
{
    if (this.count == 0)
    {
        // 何か処理。
    }
    this.count++;
}

この状態でStyleCopを実行すると2つ警告が出ます。

If文の{}後は1行開けて次の文を記述する

SA1513: Statements or elements wrapped in curly brackets must be followed by a blank line.

こいつは}の後に1行入れてあげればOKですね。

private void Button_Click(object sender, RoutedEventArgs e)
{
    if (this.count == 0)
    {
        // 何か処理。
    } // 一行改行する

    this.count++;
}

ちなみに、このメッセージはIfの}に限ってはいないのですが、特にelseなしのIf文のときはこれに従っておくと得かと思います。たとえば、

if (c1 > 0)
{
    // 何か処理
}
if (c2 == 0)
{
    // 何か処理
}
// これの間違い? 
//if (c1 > 0)
//{
//    // 何か処理
//}
//else if (c2 == 0)
//{
//    // 何か処理
//}

とあった場合でも}の後を一行あけて置く癖をつけておくと間違えようがありません。

フィールド変数はコンストラクタより上に記述する

SA1201: All constructors must be placed after all fields.

StyleCopのルールではメンバの種類ごとにまとめて記述することになっています。よって、countをコンストラクタより上に書けばOKとなります。

private int count;

public MainWindow()
{
    InitializeComponent();

    var button = new Button();
    button.Click += new RoutedEventHandler(this.Button_Click);
}

private void Button_Click(object sender, RoutedEventArgs e)
{
    if (this.count == 0)
    {
        // 何か処理。
    } // 一行改行する

    this.count++;
}

ルールに従うとソースコードはフィールド変数・コンストラクタ・イベント・プロパティ・メソッドの順番に記述することになります。どのあたりに何が書いてあるのか決まっていると、クラスを読むときに楽になりますね。図にするとこんな感じです。

フィールド変数を公開してはいけない

追加したcountフィールド変数を外部に公開したくなったとします。

// フィールドをpublicに変更
public int count;

すると

SA1401: Fields must be declared with private access. Use properties to expose fields.

と怒られます。公開したいならプロパティを使わないとだめだよ、ということですね。

public MainWindow()
{
    InitializeComponent();

    var button = new Button();
    button.Click += new RoutedEventHandler(this.Button_Click);
}

// プロパティに変更。コンストラクタの下に移動。
public int Count {get; set; }

以前、フィールドをWPFバインディングソースにしてバインディングが動かないと悩んでいる人を見かけたことがあります。フィールド変数をパブリックにしてもバインディングソースにすることができません。こういったつまらない箇所でつまずかないためにも、このルールを守ったほうが良いと思います。

public class Coordinates
{
    // こう書くとバインディングソースにできそう。
    // でもフィールドだからバインディングソースにできない。残念!
    public int X;
    public int Y;
}

usingはソートして表示する

次はusingを追加してみましょう。

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Threading; // usingの追加

すると

SA1210: Using directives must be sorted alphabetically by the namespaces.

と怒られます。usingを見ればこのクラスがどんなモジュールに依存しているのかなんとなくわかるのですが、usingの並び順が常にソートされていると読みやすさが段違いになります。というわけで、VisualStudioの「usingの並び替え」機能でソートする癖をつけましょう。

まとめ

他にもStyleCopがチェックしてくれるルールはいっぱいあるのですが、きりがないのでこの辺でやめておきます。StyleCopがどんなことをしてくれるのか?StyleCopに従うと何か良いことがあるのか?を感じ取ってもらえたならば幸いです。特に独学でC#を学んでいる人にはぜひ使ってほしいツールです。
StyleCopのルールには合う合わないがあると思います。しかし、あえてStyleCop先生のチェックに従ってみると良いエクササイズになると思います。従いたくないルールがあった場合、「StyleCopは何故このルールを導入しているのだろう?」とか考えるとよいと思います。考えた上で「俺はこのルールを破るぜ!」というならば一皮剥けたと言えるんじゃないでしょうか。