ビジュアルスタジオコードメトリックとスイッチケースの保守性インデックス
-
05-10-2019 - |
質問
ベストプラクティスに従うのが大好きな人として、
コードメトリックを実行した場合(ソリューションエクスプローラーのプロジェクト名を右クリックして、「コードメトリックの計算」 - Visual Studio 2010を選択してください)。
public static string GetFormFactor(int number)
{
string formFactor = string.Empty;
switch (number)
{
case 1:
formFactor = "Other";
break;
case 2:
formFactor = "SIP";
break;
case 3:
formFactor = "DIP";
break;
case 4:
formFactor = "ZIP";
break;
case 5:
formFactor = "SOJ";
break;
}
return formFactor;
}
それは私にの保守性インデックスを与えます 61
(もちろん、これだけがある場合、これは重要ではありませんが、クラスのようなユーティリティを使用している場合、哲学がそのようなことをしている場合、ユーティリティクラスは保守性のインデックスを最悪の状態にします。)
これの解決策は何ですか?
解決
まず、61はメンテナンス可能なコードと見なされます。保守性インデックスの場合、100はコードを維持するのが非常に簡単ですが、0はメンテナンスが困難です。
- 0-9 =維持が難しい
- 10-19 =維持するのは中程度です
- 20-100 =維持するのに適しています
メンテナビリティインデックスは、3つのコードメトリックに基づいています。つまり、Halsteadボリューム、Cyclomaticの複雑さ、およびコードの行、およびに基づいています。 フォロー式:
Max(0、(171-5.2 * ln(halstead volume)-0.23 *(cyclomatic complexity)-16.2 * ln(code of code)) * 100/171)
実際、あなたの例では、保守性インデックスの低い値の根本原因は、環状複雑さです。このメトリックは、コード内のさまざまな実行パスに基づいて計算されます。残念ながら、メトリックは必ずしもコードの「人間の読みやすさ」と一致するわけではありません。
例として、コードの結果、インデックス値が非常に低くなります(覚えておいて、維持が難しいことを意味します)が、実際には読みやすいです。これは、サイクロマティックな複雑さを使用してコードを評価する場合に一般的です。
数日間(月曜日)にスイッチブロックを備えたコードと数ヶ月間のスイッチブロック(1月= dec)を想像してください。このコードは非常に読みやすく保守可能ですが、膨大な環状複雑さをもたらし、したがってVisual Studio 2010で非常に低いメンテナビリティインデックスになります。
これはメトリックに関するよく知られている事実であり、数値に基づいてコードが審査される場合は考慮されるべきです。絶対数を見るのではなく、コードの変更の指標として理解するために、時間の経過とともに数値を監視する必要があります。たとえば、メンテナビリティインデックスが常に100で、突然10になった場合、これを引き起こした変更を検査する必要があります。
他のヒント
ソリューション用に選択している方法に拡張性がないため、保守性インデックスが高くなる可能性があります。
正しいソリューション(上記で触れられたマークシンプソン)は、コードが再構築されることなく新しいフォームファクターを使用するように拡張できるものです。コードのスイッチ /ケースステートメントは常にOOデザインが忘れられており、常に表示されるべき兆候です悪いコードの匂いとして。
個人的に、私は実装します...
interface IFormFactor
{
// some methods
}
class SipFormFactor : IFormFactor...
class DipFormFactor : IFormFactor...
Etc.
...そして、インターフェイス上のメソッドに目的の機能を提供する - あなたはそれをGOFコマンドパターンに似ていると考えることができます。
このようにして、あなたのより高いレベルの方法はただにすることができます...
MyMethod(IFormFactor factor)
{
// some logic...
factor.DoSomething();
// some more logic...
}
...そして、後日一緒に来て、ハードコーディングされたスイッチ節のようにこのコードを変更することなく、新しいフォームファクターを導入することができます。また、このアプローチは、簡単にモッキングできるため、TDDに簡単に貸してしまうことがわかります(TDDを適切に行っている場合は、これを実現する必要があります)。
この答えは非常に遅いことは知っていますが、誰もまだ辞書ソリューションを掲載していないことに興味がありました。このようにデータ指向の巨大なスイッチステートメントを扱うとき、スイッチケースを辞書に崩壊させる方が読みやすいことが多いことがわかりました。
public static readonly IDictionary<int, string> Factors = new Dictionary<int, string> {
{ 1, "Other" },
{ 2, "SIP" },
{ 3, "DIP" },
{ 4, "ZIP" },
{ 5, "SOJ" }
};
public static string GetFormFactor2(int number)
{
string formFactor = string.Empty;
if (Factors.ContainsKey(number)) formFactor = Factors[number];
return formFactor;
}
これにより、辞書へのクラス結合により、アレイソリューションよりもわずかに低い74の保守性インデックスが得られますが、通常はオンにする任意のタイプで機能するため、より一般的だと感じています。アレイソリューションと同様に、非常によくスケーリングし、多くの反復コードを削除します。
一般的に言えば、データ駆動型のアプローチを使用すると、コードがより明確になるのに役立ちます。これは、重要な部分(この場合、条件と結果)をCruft(この場合はスイッチケース)から分離するためです。
2つのことが思い浮かびます:
列挙を使用して説明と値を結婚します
public enum FormFactor
{
Other = 1,
SIP = 2,
etc.
}
クラスまたは構造体を使用して、各フォームファクターを表す
public class FormFactor
{
public int Index { get; private set; }
public string Description { get; private set; }
public FormFactor(int index, string description)
{
// do validation and assign properties
}
}
私はこのようにそれを行い、保守性インデックスを忘れます:
public static string GetFormFactor(int number)
{
switch (number)
{
case 1: return "Other";
case 2: return "SIP";
case 3: return "DIP";
case 4: return "ZIP";
case 5: return "SOJ";
}
return number.ToString();
}
読みやすく、変更しやすい私見。
どれだけ重要なのかわかりませんが、次のことは76を取得します。
private static string[] _formFactors = new[] { null, "Other","SIP","DIP", "ZIP", "SOJ"};
public static string GetFormFactor2(int number)
{
if (number < 1 || number > _formFactors.Length)
{
throw new ArgumentOutOfRangeException("number");
}
return _formFactors[number];
}
明らかに私にとって 列挙 メソッドは、ハードコーディングされた文字列が含まれないため、最も保守可能です。したがって、タイプミスの問題やコンパイル時間構文チェックはありません。制限のみがルールの命名です。