使わない引数を明示したい

Question

コーディング規約的な質問です。

今までC++でやってきたことをC#でできないか、という動機で質問します。

 

関数の引数について使わないものがある場合、

・使わないことを明示したい。

・使っておらず、↑の手段で明示してないなら、コンパイルエラーにしたい。

ということを実現したく思っています。

 

これは以下の理由によるものです。

・途中で関数の仕様やアルゴリズムを変更したことで引数がいらなくなった場合、きちんとその引数を削除することで、コード品質を高めたい。

・フレームワーク仕様で引数が変更できない場合でも、その引数は使わないと判断した、というプログラマの意志をコードで表明しておきたい。

 

ちょっと試してみたところ、C#では使ってない引数があっても警告にすらならないようですが、何かやり方が間違えているのでしょうか？

Answer 1

C#コンパイラの機能ではありませんが、コンパイル後のアセンブリ（.EXEや.DLL）に対してFxCopというツールで検査を行うことができ、その指摘事項にCA1801: 使用されていないパラメーターを再確認しますがあります。

Answer 2

もしコード分析が利用できるエディションをお使いでしたら、コード分析によって警告され、必要により抑制できます。

CA1801: 使用されていないパラメーターを再確認します
http://msdn.microsoft.com/ja-jp/library/ms182268(v=VS.100).aspx

SuppressMessageAttribute コンストラクター
http://msdn.microsoft.com/ja-jp/library/system.diagnostics.codeanalysis.suppressmessageattribute.suppressmessageattribute.aspx

Answer 3

佐祐理様、TH01様、お返事ありがとうございます。

 

内容理解し、確認自体は可能であることを理解できました。ありがとうございます。

あとはプログラマによる「この引数は使わない」という意思表明をどうするか、ですね。

 

Answer 4

どちらかというと使わない引数をもつメソッドをオーバーロードして引数を削減したバージョンを作り利用者を誘導するという事になると思います。

そのうえで過剰引数のバージョンに Obsolete 属性を付与すれば呼び出し側に警告がかかります。

 

public Hoge DoSomething( Hoge hoge, Hage hage )
{
    …
}

で hage が不要だとなった場合、以下のコード断片を適用します。

[Obsolete("hage was not used by this method. call DoSomething( hoge )")] // <- insert fragment
public Hoge DoSomething( Hoge hoge, Hage hage )
{ // insert fragment begin
    DoSomething( hoge )
}
public Hoge DoSomething( Hoge hoge )
{  // insert fragment end
    …
}

Obsolete はメッセージに加えて error と扱うかの boolean 値を持ち、指定しなかったもしくは false 時には warning になりますので、本気に廃止する事ができると判断できたら error にする様にして強制移行という事になるでしょう。

 

---

Kazuhiko Kikuchi

Answer 5

kazuk様

お返事ありがとうございます。

 

今回のプロジェクトは引数を即座に削除してビルドエラーにしてよい性質のものですので、無駄な引数を持つ関数の利用者側よりは実装者側の問題に対する解決策を求めています。

 

Obsolete属性というのは知りませんでした。これは色々と使えそうなので覚えておきます。ありがとうございます。

Answer 6

> 無駄な引数を持つ関数の利用者側よりは実装者側の問題に対する解決策を求めています。

これは

> あとはプログラマによる「この引数は使わない」という意思表明をどうするか、ですね。

ですよね？ その回答として Obsolete による overload では、どのような問題がありますか？

// 旧 int method1(int a, int b, int c) { return a + b + c; } // 新 [Obsolete("パラメータ C は常に 0 が渡され、結果に影響しないため使わなくなりました。", true)] int method1(int a, int b, int c) { return method1(a, b); } int method1(int a, int b) { return a + b; }

このような変更をすることで

• メソッドの実装者が、新しいロジック側で消失したパラメータを誤って使用していた場合に、実装者側でコンパイルエラーとなる。
• メソッドの利用者が、古いメソッドの呼び出しを放置した場合に不要なパラメータを通達できる。
• メソッドの利用者が、古いメソッドの呼び出しを放置した場合にコンパイルエラーにできる。（警告にもできる）

ということで、目的はおおむね果たされているかと思います。

 

Answer 7

// 旧
int method1(int a, int b, int c)
{
    return a + b + c;
}

// 新
[Obsolete("パラメータ C は常に 0 が渡され、結果に影響しないため使わなくなりました。", true)]
int method1(int a, int b, int c)
{
    return method1(a, b);
}

int method1(int a, int b)
{
    return a + b;
}

こーどがおかしいので再掲（編集すると改行が増えるかわりに、改行が消滅するようになった？）

Answer 8

K.Takaoka様

お返事ありがとうございます。

 

>> あとはプログラマによる「この引数は使わない」という意思表明をどうするか、ですね。

>その回答として Obsolete による overload では、どのような問題がありますか？

 

意思表明をする必要があるのは、メソッドの利用者ではなく、実装者です。

 

まず前提として

>今回のプロジェクトは引数を即座に削除してビルドエラーにしてよい

 のです。

自分で設計したメソッド/インターフェースなら、使われなくなった引数は削除して問題ないのです。

だから引数を削除すればその関数のコール箇所もビルドエラーになり、修正すべき箇所もわかります。わざわざObsolete属性を使う場面ではないと思います。

 

「この引数は使わない」と明示する必要があるのは、引数を自分で削除できないケースです。例えば

• GUIコントロールのイベントハンドラ等、フレームワーク側で引数が決められている
• Strategyパターンなどで、ストラテジAでは使う引数について、ストラテジBでは使わない

などになります。