雑記

サイクロマティック複雑度を低くして品質を向上させる方法

2023/1/8

１．概要
２．題材
３．機能分割
４．ポリモーフィズムを適用する
５．実装分割
６．小技集と考察
改訂履歴

１．概要

この記事ではサイクロマティック複雑度を下げるという観点で、プログラムの品質を向上させる具体的なプログラミング方法を説明します。

サイクロマティック複雑度は、簡単に言うとプログラムの複雑性を示す指標で低い方が優秀です。高くても１メソッド当たり10～15におさめるべきとされています。

メソッドのサイクロマティック複雑度を下げるポイントは、クラスやメソッドを分割することです。

メソッドやクラスの数が増えても、その中身の複雑度が下がれば全体の品質が向上するということを説明します。

この記事ではメソッドを分割する方法として２つを説明します。

機能に着目した機能単位での分割と、ポリモーフィズムの観点で実装に着目した実装単位での分割です。

特に後者のポリモーフィズムの観点での分割は、いろいろなところで言及されたりほのめかされたりするものの、具体例が説明される機会がほとんどないようなのでこの記事が参考になればと思っています。

この記事ではまず、保守容易性指数が 68 である題材のサンプルプログラムを紹介します。

題材はC#のプログラムですが、この記事の考え方はほとんどのオブジェクト指向のプログラミング言語で当てはまります。

この題材を２通りの方法で分割し、品質が向上することを示し、その方法や考え方、注意点などを具体的に記載します。

先に結果を紹介しておくと次のように品質が向上します。

	全体の保守容易性指数	全体の実行可能コードの行数	最も複雑なメソッドのサイクロマティック複雑度	全体のクラス数	全体のメソッド数
題材	68	18	4	1	3
機能分割版	76	22	3	1	6
実装分割版	84	26	2	3	11

最初に、題材を機能単位で分割した時点で、保守容易性指数が 76 に向上、サイクロマティック複雑度が 4 から 3 に改善することを示します。

その後で、さらに実装単位で分割して、保守容易性指数が 84 に向上、サイクロマティック複雑度が 2 まで改善することを示します。

保守容易整数指数はサイクロマティック複雑度やコードの行数・演算処理の量などを総合的に判断する指数で、高い方が優秀で100が最高値です。

つまり、うまく分割すればクラス数やメソッド数が増えても総合的に品質が良いということが示されます。

→ 参考：コードメトリック - 保守容易性指数の範囲と意味

最後には補足で、サイクロマティック複雑度に関する小技やちょっとの考察を記載します。

サイクロマティック複雑度に関して簡単な説明が欲しい方はこちらを参照してください。

コードメトリック - サイクロマティック複雑度

もっと詳しく知りたい方は下記レポートを参照してください。

Structured Testing: A Testing Methodology Using the Cyclomatic Complexity Metric

２．題材

この記事では下記のC#のサンプルを題材にサイクロマティック複雑度を下げる方法を説明します。

このメソッドは製品情報をデータベースに登録します。新しい製品であれば普通に製品情報テーブルに追加するだけです。既存の製品の変更登録であれば、変更前の製品情報を論理削除し、変更後の製品情報を追加するという仕様です。これだけだとシンプル過ぎてサンプルにならないので、登録済み製品で製品種別が"D"の場合は、何か追加の処理が必要で価格が10000より大きい場合とそうでない場合とで違う処理が必要になるという仕様にしました。

public void RegisterProduct(Product product)
{
    //トランザクション開始
    using SqlConnection cn = Database.NewConnection();
    using SqlTransaction tran = cn.BeginTransaction();

    //この製品を検索
    SqlCommand checkSql = cn.CreateCommand();
    checkSql.CommandText = "SELECT seq FROM product WHERE productid = @productid AND deleteflag = 0";
    checkSql.Parameters.Add(new SqlParameter("@productid", product.id));

    int? seq = (int?)checkSql.ExecuteScalar();

    //この製品が登録済みか判断
    if (seq != null)
    {
        //検索にヒットした場合(=既存製品の場合)
        //まず、登録済み情報を論理削除
        SqlCommand deleteSql = cn.CreateCommand();
        deleteSql.CommandText = "UPDATE product SET deleteflag = 1 WHERE seq = @seq";
        deleteSql.Parameters.Add(new SqlParameter("@seq", seq.Value));

        //製品種別がDの場合は、さらに追加処理が必要。
        if (product.productType == "D")
        {
            if (product.price < 10000)
            {
                SpecialDeleteOperationLower(); //価格が10000より小さい場合の追加処理
            }
            else
            {
                SpecialDeleteOperationHigher(); //価格が1000以上の場合の追加処理
            }
        }

    }

    //新しい製品の登録
    SqlCommand registerSql = cn.CreateCommand();
    registerSql.CommandText = "INSERT product VALUES (...)";
    registerSql.ExecuteNonQuery();

    //トランザクション終了
    tran.Commit();
}

public void SpecialDeleteOperationLower()
{
    //価格が10000より小さい場合の追加処理。このサンプルでは何もしません。
}

public void SpecialDeleteOperationHigher()
{
    //価格が1000以上の場合の追加処理。このサンプルでは何もしません。
}

このプログラムのコードメトリックスは次の通りです。

	全体の保守容易性指数	全体の実行可能コードの行数	最も複雑なメソッド	最も複雑なメソッドのサイクロマティック複雑度
題材	68	18	RegisterProduct	4

参考

この記事ではプログラムの構造上の複雑さを扱いたいので、実際にビルドしたり実行できる必要はありませんが、C#でやってみたい人のためにビルドできるようにする手順を示しておきます。この部分はこの記事の内容を理解するのに必須ではないので興味がない人はスキップしてください。

C# のクラスライブラリのプロジェクト SampleCommon を作成して、NuGet で Microsoft.Data.SqlClient をインストールします。下記のプログラムをコピーします。

using Microsoft.Data.SqlClient;

namespace SampleCommon
{
    public class Product
    {
        public string id { get; set; }
        public int price { get; set; }

        public string productType { get; set; }
    }

    public class Database
    {
        public static SqlConnection NewConnection()
        {
            throw new NotImplementedException();
        }
    }
}

次にそれとは別にC# でクラスライブラリのプロジェクト Sample1 を作成して、NuGet で Microsoft.Data.SqlClient をインストールします。このプロジェクトのClass1内にに題材で示したRegisterProductメソッドを含むプログラムをコピーします。プログラムの先頭に using Microsoft.Data.SqlClient; と using SampleCommon; を追記します。

それとは別にクラスライブラリのプロジェクト SampleCommon を作成し、下記のプログラムをコピーします。

Sample1プロジェクトにSampleCommonプロジェクトへのプロジェクト参照を追加します。

３．機能分割

３－１．１メソッド１機能になるように分割する

メソッドの複雑度を減らす最も効果的な方法は、メソッド行数を減らすことです。

メソッドの行数が無駄に多くなっていることは少なくなりません。その理由は１つのメソッドに複数の機能を実装するからです。

基本的には、１つのメソッドに１つの機能だけを実装するようにしてください。あなたが作っているメソッドの機能は何でしょうか？

この質問の答えが１つであれば多分大丈夫です。あれをやってそれをやりますのように複数の答えがあるのあら、その「あれ」と「それ」を別のメソッドに分離できないか考える必要があるかもしれません。

題材にしているREgisterProductメソッドは、まさにその「あれ」もするし「それ」もするというメソッドの例です。

このメソッドは、検索と論理削除(製品種別"D"の追加処理含む)と、新しい製品の登録処理をやっています。機能が多いですね。

この３つの機能をそれぞれ別メソッドに切り出してみましょう。

本体は次のようになります。

public void RegisterProduct(Product product)
{
    //トランザクション開始
    using SqlConnection cn = Database.NewConnection();
    using SqlTransaction tran = cn.BeginTransaction();

    //この製品を検索
    int? seq = GetSeq(product.id, cn);

    if (seq != null)
    {
        //検索にヒットした場合(=既存製品の場合)、論理削除処理
        DeleteLogical(seq.Value, product, cn);
    }

    //新しい製品の登録
    Add(product, cn);

    //トランザクション終了
    tran.Commit();
}

検索処理は GetSeqメソッドとしました。

public int? GetSeq(string productid, SqlConnection cn)
{
    SqlCommand checkSql = cn.CreateCommand();
    checkSql.CommandText = "SELECT seq FROM product WHERE productid = @productid AND deleteflag = 0";
    checkSql.Parameters.Add(new SqlParameter("@productid", productid));

    int? seq = (int?)checkSql.ExecuteScalar();
    return seq;
}

論理削除処理は DeleteLogicalメソッドとしました。

public void DeleteLogical(int seq, Product product, SqlConnection cn)
{
    //まず、登録済み情報を論理削除
    SqlCommand deleteSql = cn.CreateCommand();
    deleteSql.CommandText = "UPDATE product SET deleteflag = 1 WHERE seq = @seq";
    deleteSql.Parameters.Add(new SqlParameter("@seq", seq));

    //製品種別がDの場合は、さらに追加処理が必要。
    if (product.productType == "D")
    {
        if (product.price < 10000)
        {
            SpecialDeleteOperationLower(); //価格が10000より小さい場合の追加処理
        }
        else
        {
            SpecialDeleteOperationHigher(); //価格が1000以上の場合の追加処理
        }
    }
}

新しい製品の登録処理はAddメソッドとしました。

public void Add(Product product, SqlConnection cn)
{
    //新しい製品の登録
    SqlCommand registerSql = cn.CreateCommand();
    registerSql.CommandText = "INSERT product VALUES (...)";
    registerSql.ExecuteNonQuery();
}

３－２．機能分割の効果

これでメトリックスがどう改善されたか見てみましょう。

	全体の保守容易性指数	全体の実行可能コードの行数	最も複雑なメソッド	最も複雑なメソッドのサイクロマティック複雑度	全体のクラス数	全体のメソッド数
題材	68	18	RegisterProduct	4	1	3
機能分割版	76	22	DeleteLogical	3	1	6

保守容易性指数が68から76に向上しています。つまり、機能分割版の方が総合的に品質が良いということが示されています。

最も複雑なメソッドのサイクロマティック複雑度は4から3に減ったことからも品質が改善されていることがわかります。

とはいえ、題材のサイクロマティック複雑度が 4 なのであまり改善された実感はないかもしれません。

コードの行数とメソッドの数は増えていますが、これが品質に与えるインパクトより、サイクロマティック複雑度が減ったことのインパクトの方が大きいというがわかります。

サイクロマティック複雑度がたった１さがっただけでもVisual Studioは分割したほうが優秀と判断するのですから、サイクロマティック複雑度100のメソッドをサイクロマティック複雑度10の複数のメソッドに分割できるようであればすばらしい効果がありますし、実感としてもシンプル化された効果を感じられると思います。

サイクロマティック複雑度はプログラミング構造上のホワイトテストのテストケース数を示しています。テストの要因にはプログラミングの構造に表されない要素、たとえば、はデータベースの内容や、クッキーの内容、ブラウザーの種類、ファイルの内容などもあるので、理論上のテストケースの数は、サイクロマティック複雑度 × プログラミング構造外の要因になります。だから、サイクロマティック複雑度が1違うということは、かけ算の数字を１つ減らせるという意味になります。一方、メソッドが増えるということはテストケース数はメソッド１のテストケース数+メソッドの２のテストケース数のようにたし算で増えます。つまり、サイクロマティック複雑度を減らしてメソッドを増やすということは、かけ算からたし算に複雑度を転換しているようなイメージです。

※このかけ算からたし算への転換という理屈付けは、私が独自に書いているものであり、他の出典はないし、誰かが言っているのを聞いたこともありません。ですから、この話の信頼度は茶飲み話くらいで受け取ってもらった方が良いかもしれません…。

３－３．メソッドの分割の考え方

１つのメソッドがコメントや空白行を除いて30行以上あるようならば、そのメソッドは分割の候補です。複数の機能が実装されているかもしれません。

30行程度であれば、そのメソッドを見て機能を理解するのに困難はなく、どんなに下手にプログラムしてもそうそう複雑性も増えないはずです。

この30行という基準は、私が経験的に言っているだけの値ですので、違う基準値を設けても良いです。が、100行を超える基準値はおかしいと言っておきます。なお、開発ルールとしてこのような基準を設けるときは、機械的に判断せず、行数が多い代わりに得られる何かの方に価値があれば、柔軟に判断するようにするのが良いと持っています。

それから、よく「共通で呼び出される処理はメソッドとして分割しておく」と言います。確かにそうすると同じ処理を何度も書かなくてよくなるのでとても効果があります。

しかし、上述のように、メソッドを作成する理由は共通処理だけではありません。

たった１か所からしか呼び出されないメソッドであっても、作成することで保守性を高める効果があるということを上の例で紹介しました。

共通処理ではなくても遠慮なくメソッドを作成してください。共通であるかないかにかかわらず１つのメソッドは１つの機能だけを実行するのが理想です。

４．ポリモーフィズムを適用する

４－１．ポリモーフィズムで条件分岐をなくす例

プログラム内の条件判断を減らすには、ポリモーフィズムの観点で処理を分割できないか検討してください。

こう書くと、ほとんどの人には意味が通じず、話を聞いてもらえないので、題材を扱う前に小さな具体例で説明することにします。「ポリモーフィズム」が何かを理解していなくてもここで説明する内容は理解できると思います。

if文などの構文での条件分岐とポリモーフィズムを適材適所で使い分けます。何でもかんでも if文などの構文上の条件分岐が良いわけではありません。逆にポリモーフィズムがすべてを解決するわけでもありません。両方を理解して使い分けるのがベストです。

ここで紹介する例は C# ですが、オブジェクト指向言語であれば、言語は限定されないはずです。VBやPythonやJavaScriptでも実装方法は異なりますが考え方は通用します。

この例のWriteMessageメソッドはログ出力の共通ロジック風のプログラムです。

public class Class1
{
    public void Test()
    {
        //呼び出し例
        Logger logger = new Logger();
        logger.WriteMessage("テスト", 1, true);
    }
}

public void WriteMessage(string message, int writingType, bool addition)
{
    if (writingType == 0) //writingType == 0 の場合は、デバッグ出力します。
    {
        Debug.WriteLine(message);
    }
    else if (writingType == 1) //writingType == 1 の場合は、テキスト出力します。
    {
        if (addition) //additionの場合は、追加書き
        {
            File.AppendAllText(@"C:\temp\system.log", message);
        }
        else //additionがfalseなら上書き
        {
            File.WriteAllText(@"C:\temp\system.log", message);
        }
    }
    else if (writingType == 2) //writingType == 2の場合は、標準出力に出力します。
    {
        Console.WriteLine(message);
    }
}

このプログラムは、else if も含めると if で4回条件分岐しています。この条件分岐なしで同じことを実現するにはどうしたらよいのでしょうか？

条件分岐をなくしてサイクロマティック複雑度の低いプログラムに書き換えてみましょう。

このプログラムはログを出力するだけですが、４つのログの出力方法があるため、if で分岐しています。「ログを出力する」という動作が共通していて実装が異なるということですから、ポリモーフィズムの考えが適用できます。

そこで、この「ログを出力する」という共通の操作を定義するインターフェースを記述し、実装は４つのクラスのそれぞれ分けます。

次のように書き換えられます。

public class Class1
{
    public void Test()
    {
        //呼び出し例
        ILogger logger = new AddionalFileLogger();
        logger.WriteMessage("テスト");
    }
}

public interface ILogger
{
    void WriteMessage(string message);
}

public class DebugLogger : ILogger
{
    public void WriteMessage(string message) => Debug.WriteLine(message);
}

public class AddionalFileLogger: ILogger
{ 
    public void WriteMessage(string message) => File.AppendAllText(@"C:\temp\system.log", message);
}

public class OverwriteFileLogger : ILogger
{
    public void WriteMessage(string message) => File.WriteAllText(@"C:\temp\system.log", message);
}

public class ConsoleLogger : ILogger
{
    public void WriteMessage(string message) => Console.WriteLine(message);
}

見ればわかるように、分割された各メソッドは非常にシンプルで、サイクロマティック複雑度は1です。

分割前後のメトリックスの比較は次の通りです。

	全体の保守容易性指数	全体の実行可能コードの行数	最も複雑なメソッド	最も複雑なメソッドのサイクロマティック複雑度	全体のクラス数	全体のメソッド数
分割前	75	10	WriteMessage	5	2	2
分割度	96	6	(すべて同じ)	1	6	6

保守容易性指数とサイクロマティック複雑度が向上しているのがわかります。

機能分割の例と同じようにメソッド数は増えました。また、今回はクラス数も増えています。それでも、プログラムの品質が向上するということです。

※ここではインターフェースもクラスにカウントしています。

呼び出し側は次のようになっています。これもサイクロマティック複雑度1です。ifであれだけ分岐していたのに、分岐など１つもなくすことができました。

ILogger logger = new AddionalFileLogger();

logger.WriteMessage("テスト");

この例では、WriteMessageを呼び出す直前でnewしていますが、どこか１か所でnewしたものを使いまわせばより効率的ですし、依存性注入でDIコンテナなどにもつながる構造になりました。

４－２．実装の分割に複雑度を下げる効果があるか

いや、ちょっと待てと。この書き換えに首をひねる人もいることでしょう。

書き換え後のロジックで、ログ出力方法を変更しようと思ったら、今度は呼び出し側で分岐が必要になり、呼び出し側のサイクロマティック複雑度が上がることになるから、トータルで見れば意味がないのでないでしょうか？

実は呼び出し側での分岐はまず必要ありません。

たとえば、書き換え前のロジックで引数をで区別していたことが、クラスで区別するように変わるだけだからです。

もし、書き換え後の呼び出し側をこのようにイメージしているとしたらそれは違います。

//変更後の、呼び出し側の間違ったイメージ
ILogger logger;

if (writingType == 0)
    logger = new DebugLogger();
else if (writingType == 1 && addition)
    logger = new AddionalFileLogger();
else if (writingType == 1 && !addition)
    logger = new OverwriteFileLogger();
else
    logger = new ConsoleLogger();

logger.WriteMessage("test");

このイメージでは、writingType や addition という変数がどこかから与えられてそれで条件分岐することになっていますが、この変数はどこから発生するのでしょうか？

呼び出し側のさらに呼び出し元でしょうか？では、それをさかのぼっていくと根本的には何に由来するのでしょうか？

大元では、writingType や addition などの条件は固定で記述されているか、設定ファイルやなにかプログラム外部から読み取っているはずです。

固定で記述されているのであれば、その大元で変数など使わずに ILogger のインスタンス(たとえば、変数名logger)を生成して、それを他の箇所から参照できるようにしたり、引数で渡すようにすればよいのです。そうすれば、ログを出力したい個所では、それが何であれ logger.WriteMessage を呼び出せばよいだけです。条件分岐など必要ありません。(これがポリモーフィズムの醍醐味です。)

設定ファイルなどの外部のなにかの要因でログ出力方式が変わるようになっているのなら、その条件に応じて対応するインスタンスを生成する必要があるので、たしかに大元で１回は条件分岐をする必要があるかもしれません。１つの解決策は依存性注入の使用です。インターフェースを介して操作が呼び出されるようになっていれば、依存性注入を利用して外部から使用すべきインスタンスを指示することができます。こうするとプログラム内で条件分岐する必要がなくなります。

もう１つの解決策、というか考え方は、それはOKとするものです。インスタンス生成の部分での条件分岐は基本的には問題ありません。インスタンス生成のための条件分岐程度は、多くてもサイクロマティック複雑度15以内には収まるでしょう。このコストを支払えば、ログ書き込みの実装は各クラスに分散されるので、その実装である程度分岐が発生しても複雑度が低く抑えやすくなります。これは、この記事の最初に紹介した複数の機能をもつメソッドを単機能のメソッドに分割して複雑が下がったのと同じ発想です。

４－３．私のプログラムには適用できないという罠

実践の壁

私の経験上、このような説明をしても、自分のプログラムには適用できないと主張するプログラマーがいます。「確かに、この例で実装の分割を行って複雑度を下げられることはわかった。でも、私が担当しているプログラムは仕様上どうしても外せない条件が複数あり、ログ出力のように単純ではないので、ポリモーフィズムは適用できそうにない。」というような主張です。

確かにそのような場合もありますが、多くの場合、これは、理解や経験の不足からくる間違いです。

どうすればポリモーフィズムをうまく適用できる場合と、そうでない場合を区別できるでしょうか？

これにちゃんと回答しようとすると、そもそもポリモーフィズムやプログラムの構造化やオブジェクトの考え方などいろいろなことを説明する必要があります。かなりのボリュームになります。

そこで、不完全ではありますが、ガイドラインのような、なにかヒントらしきものに少し挑戦してみることにします。

全体的に考える

まず、問題となっているメソッドの観点からだけ考えても解決しません。上記で紹介した例のように、呼び出し元やそのメソッドがかかわる機能の実装のデザイン(上記の例でいうと、そもそもログ出量方式が決定する大元は何か、依存性注入は適用できないのか、インスタンス生成方法と実装を分離できないかなど)を含めて考えなければポリモーフィズムは実現できません。そのメソッド「だけ」ポリモーフィズムを使って書き換えてうまいこと複雑度を下げるという発想ではうまくいきません。関連する機能全体を書き換えるくらいの気持ちで考えてください。

同じ機能で実装が異なる

それから、分岐している理由が、同じ機能だけど実装が異なるからと考えてみてください。たとえば、同じ登録処理なんだけれども、ある条件の場合はこうなり、別の条件のば場合はこうなるという分岐の場合は、その２つを別々の登録処理として実装し、ポリモーフィズムを適用することを検討します。業務システムで使用する多くの処理は類型化されており、このような「同じ○○なんだけど、事情があって、その中でいろいろ分岐が必要」ということが多々あります。類型化されている業務機能の例は「売る」「買う」「出荷する」「請求する」「計上する」「申請する」など多数です。もっと細かい粒度で考えてもよいです。

商品によって処理が異なる場合は、商品の方をポリモーフィズムの観点で整理した方が良いかもしれません。このような観点での設計をオブジェクト指向設計と呼びますが、あまりオブジェクト指向にこだわって頭でっかちなプログラムなるよりは、サイクロマティック複雑度を軽減することを具体的な目標としながら、プログラムのシンプル化を目指す方が地に足がついた対応ができるかもしれません。

次の章では最初に出てきた製品登録の題材をポリモーフィズムの観点で実装分割する例を紹介します。

５．実装分割

５－１．同じ機能の実装の違いで分割する

最初の題材に挙げた製品登録のサンプルでは、製品種別(productType)が"D"の製品だけ、少し特別扱いをする if 文があります。ポリモーフィズムの観点で通常の製品を扱うクラス(ProductManagerクラス)と、製品種別Dの製品を扱うクラス(TypeDProductManagerクラス)を分割します。これによりそのif文は不要になります。

その代わりに、処理を開始する前に、ProductManagerクラスかTypeDProductManagerクラスかどちらのインスタンスを生成するか条件判断する必要があります。これを FromProductメソッドで実施しています。上記で説明したように、インスタンス生成の分岐は複雑度が高くならないので許容し、実装が分離されていることで複雑度を低減する大きな価値があります。

TypeDProductManagerはProductManagerを継承します。これにより違いがある部分の処理だけ記述すればよくなります。

ProductManager クラス

public class ProductManager
{
    public Product Product { get; set; }
    public int? seq { get; set; }

    /// <summary>
    /// 製品からインスタンスを生成します。
    /// productTypeが D の場合だけ、TypeDProductManager を生成します。
    /// </summary>
    public static ProductManager FromProduct(Product product)
    {
        if (product.productType == "D")
            return new TypeDProductManager(product);
        else
            return new ProductManager(product);
    }

    protected ProductManager(Product product) => this.Product = product;

    /// <summary>このProductがデータベースに存在するかを示します。</summary>
    public bool Exists { get => this.seq != null; }

    /// <summary>データベースからProductのSEQを取得します。</summary>
    public void FillSEQFromDatabase(SqlConnection cn)
    {
        SqlCommand checkSql = cn.CreateCommand();
        checkSql.CommandText = "SELECT seq FROM product WHERE productid = @productid AND deleteflag = 0";
        checkSql.Parameters.Add(new SqlParameter("@productid", Product.id));

        this.seq = (int?)checkSql.ExecuteScalar();
    }

    /// <summary>この製品を登録します。すでに登録済みのものがあれば論理削除します。</summary>
    public void Register(SqlConnection cn)
    {
        if (!this.Exists)
        { 
            this.DeleteLogical(cn);　//登録済み製品であれば、まず論理削除
        }

        this.Add(cn); //新しい製品の登録
    }

    /// <summary>単純に新しい製品を登録します。</summary>
    protected virtual void Add(SqlConnection cn)
    {
        //新しい製品の登録
        SqlCommand registerSql = cn.CreateCommand();
        registerSql.CommandText = "INSERT product VALUES (...)";
        registerSql.ExecuteNonQuery();
    }

    /// <summary>製品を論理削除します。</summary>
    public virtual void DeleteLogical(SqlConnection cn)
    {
        SqlCommand deleteSql = cn.CreateCommand();
        deleteSql.CommandText = "UPDATE product SET deleteflag = 1 WHERE seq = @seq";
        deleteSql.Parameters.Add(new SqlParameter("@seq", this.seq.Value));
    }
}

TypeDProductManagerクラス

public class TypeDProductManager: ProductManager
{
    public TypeDProductManager(Product product) : base(product) { }

    public override void DeleteLogical(SqlConnection cn)
    {
        base.DeleteLogical(cn);

        //製品種別がDの場合は、さらに追加処理が必要。
        if (Product.price < 10000)
        {
            SpecialDeleteOperationLower(); //価格が10000より小さい場合の追加処理
        }
        else
        {
            SpecialDeleteOperationHigher(); //価格が1000以上の場合の追加処理
        }
    }

    public void SpecialDeleteOperationLower()
    {
        //価格が10000より小さい場合の追加処理。このサンプルでは何もしません。
    }

    public void SpecialDeleteOperationHigher()
    {
        //価格が1000以上の場合の追加処理。このサンプルでは何もしません。
    }
}

呼び出し元クラス(Class1とします)

public class Class1
{
    public void RegisterProduct(Product product)
    {
        //トランザクション開始
        using SqlConnection cn = Database.NewConnection();
        using SqlTransaction tran = cn.BeginTransaction();

        ProductManager manager = ProductManager.FromProduct(product);
        manager.FillSEQFromDatabase(cn);
        //新しい製品の登録
        manager.Register(cn);

        //トランザクション終了
        tran.Commit();
    }
}

５－２．実装分割の効果

これでメトリックスがどう改善されたか見てみましょう。

	全体の保守容易性指数	全体の実行可能コードの行数	最も複雑なメソッド	最も複雑なメソッドのサイクロマティック複雑度	全体のクラス数	全体のメソッド数
題材	68	18	RegisterProduct	4	1	3
機能分割版	76	22	DeleteLogical	3	1	6
実装分割版	84	26	DeleteLogicalなど	2	3	11

保守容易性指数が84にまで向上しています。最も複雑なメソッドのサイクロマティック複雑度も機能分割版からさらに減って2です。に減ったことからも品質が改善されていることがわかります。

メソッド数は増えましたが、多くのメソッドは行数も少なく上から順番に命令を実行していくだけのシンプルな構造になっています。いくつかのメソッドでは条件分岐が１回だけ登場します。

５－３．オブジェクト指向の罠

「そうかそうか、結論から言うと、オブジェクト指向でポリモーフィズムすれば品質がよくなるのか。」と理解した人はちょっと注意してください。

オブジェクト指向はそれはそれで複雑で、使いどころを間違うとサイクロマティック複雑度を減少させることができても、保守容易性指数が悪くなります。下手にオブジェクト指向を使うくらいなら、素直に上から順番に処理の流れを追っていけるようなプログラムの方が理解しやすい(がゆえに、品質も良く感じる)ということがしばしばあります。

特に私は依存性注入などを活用してあちこちで実装が抽象化されているようなプログラムは保守性が悪いなと感じています。プログラムを見てもどこで何をやっているかわからなくなるのですよね。

慣れないうちは、全面的にオブジェクト指向を適用しないように気を付けてください。特に、ただ単位共通の処理があるという理由でクラスに継承関係を作らないでください。

もし、あなたが作っているプログラムがちょっとしたスクリプトレベルで、全体のコード行数も大したことがないのだれば、オブジェクト指向を適用する誘惑にあらがって、上から順番に目で処理を追っていけるプログラムの方が保守性が高く、次の担当者に引き継ぎやすいかもしれません。

今回題材としたプログラムはもともとサイクロマティック複雑度が低いので特にそうなのですが、実装分割をしたバージョンと、はじめの題材のバージョンを見比べてみると、どちらが理解しやすいかという言うと、はじめの題材のバージョンだと感じる人が多いのではないでしょうか？

今回説明したようなポリモーフィズムの適用はもっと規模が大きく複雑なプログラムを念頭においており、この記事は、あくまで説明用にかなりシンプルな題材を利用しているだけです。

オブジェクト指向はこの記事の本題ではないのでここでは簡単な注意喚起に留めておきます。

６．小技集と考察

６－１．無意味な分岐を減らす

プログラム内に無意味な分岐が埋め込まれている場合があります。

たとえば、次のような例です。if での分岐が２か所にあるので、サイクロマティック複雑度は3です。

if (count >= 10)
{
    //省略1
}
else if (count < 10)
{
    //省略2
}

この例では else if の方は、単に else に書き換えるべきです。こうするとサイクロマティック複雑度は2になります。

if (count >= 10)
{
    //省略1
}
else
{
    //省略2
}

この書き換えでは本質的な複雑度は何も変わっていませんが、プログラムの理解しやすが向上し、メトリックスがより実体を反映した正確な値になります。

６－２．分岐の配列化

単に値を取得すればよいだけの分岐の場合、分岐を使わずに書き換えることができます。

次のプログラムはその例です。この例のサイクロマティック複雑度は4です。

if (month == 1)
    monthName = "January";
else if (month == 2)
    monthName = "February";
else if (month == 3)
    monthName = "March";

このプログラムは次のように配列を使って書き換えることができて、なんとサイクロマティック複雑度は1になります。

string[] monthNames = {"January", "February", "March"};

monthName = monthNames[month - 1];

条件が文字などで配列にできない場合は、配列の代わりにDictionaryなどが使用できます。

この書き換えで本質的にプログラム品質が向上しているのかどうか、私は答えを持っていません。

値を取得するだけの条件分岐は、本来の複雑性を構成する条件分岐構造とは質的に異なるものと解釈できるのであれば、意味のある書き換えです。

が、印象としては小手先で見かけの数値だけ改善しているようにも見えます。

サイクロマティック複雑度の良いところは、それが客観的な数値で表現されることです。だから、もし、この書き換えに意味がないのだとすれば、その理由にあーだこーだと理屈を並べるのではなく、数値で示さなくてはいけないのかなと感じています。

６－３．switch文の複雑性

「サイクロマティック複雑度が高くても、私のswitch文は、わかりやすく整理されているから大丈夫」と思う人が時折います。

switch文に限った話ではないのですが、次ように柵上に分岐しているケースです。

switch (denpyoKubun)
{
    case Denpyo.Nyukin:
        //入金伝票用のロジック
        break;
    case Denpyo.Shukkin:
        //出金伝票用のロジック
        break;
    case Denpyo.Furikae:
        //振替伝票用のロジック
        break;
    case Denpyo.Shiire:
        //仕入伝票用のロジック
        break;
    case Denpyo.Uriage:
        //売上伝票用のロジック
        break;
}

このswitch文だけでサイクロマティック複雑度は6です。

それぞれのロジックの中でいろいろ分岐するとどんどんサイクロマティック複雑度が増えていくのですが、caseごとに明確にロジックが分かれているので、このような場合は、メソッド単位でのサイクロマティック複雑度を問題にするのではなく、各case内のサイクロマティック複雑度を問題にすれば良いというような理屈です。

この議論は1996年のレポートに既に登場するので、少なくとも30年くらい前からあるようです。

どう考えるのが良いのか私にはわかりませんが、このようなロジックの多くはポリモーフィズムの観点で書き換えられるのではないかと感じています。

また、このようなロジックが残る場合でも、それぞれのcase内のロジックを別メソッドに分離することは簡単なことなので、そのような工夫はするべきだと思います。

この例ではcaseの分岐が5個しかありませんが、もし、何か50種類で分岐が必要にあったら、このswitch文だけでサイクロマティック複雑度が50になります。たしかに、このswtich文だけでもテストしようと思ったら50個のテストケースが必要です。もし、私がリーダーだったら、このようなswitch文が必要な設計は可能であれば避けようとすると思いますが、特別にこだわるような悪いプログラムでもないようにも思います。