C#でドメイン駆動をする前に良いコードと悪いコードの定義を理解しよう!2

当サイトではアフィリエイト広告を利用しています。

C#でドメイン駆動開発

これからC#でドメイン駆動開発とテスト駆動開発をつかってプログラミングをしていきますが,その前にそもそも良いコードと悪いコードがどういったものかを整理しましょう。悪いコードがどういったものかがわからないと悪いコードはなくなりません。ここでは悪いコードを見極める指標を解説します。

良いコード,悪いコード

良いコードと悪いコードの指標はテスト容易性と保守性であるといいました。まずは悪いコードを見ていくことで,悪いコードのどこが悪いのかを検証していきましょう。

悪いコード

まず,悪いコードができるまでの過程を考えましょう。おそらく誰にでも身に覚えがあると思います。例えば「表示ボタンを押したら,データベースの計測テーブルより,最新の計測値を取得して画面に表示する」という仕様だったとしましょう。オブジェクト指向もMVVMもすべて忘れて言語仕様しか知らないとしましょう。

言語仕様とはif文はどう書くか?
変数の宣言はどうする?
というコンパイルを通すための最低限の知識です。

この知識なしではプログラミングできませんので,プログラムを書く人はまずこの知識を身に着けます。しかし,その知識しか身に着けなかった場合どのようになるかを考えましょう。

まず,「表示ボタンを押したら計測値を表示する」という仕様ですから「表示ボタン」と「計測値」ラベルを画面に張り付けますね。

コントロールに名前を付けたらボタンクリックイベントを生成します。

ボタンクリックイベントではデータベースにアクセスし,ラベルのテキストに代入します。

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }
    
    private void DataButton_Click(object sender, EventArgs e)
    {
        string sql = @"select top 1 MeasureValue from Measures order by MeasureDate desc";
        var dt = new DataTable();
        using (var connection = new SqlConnection("XXXXXXここは接続先情報XXXXXX"))
        using (var adapter = new SqlDataAdapter(sql, connection))
        {
            connection.Open();
            adapter.Fill(dt);
            if (dt.Rows.Count > 0)
            {
                this.MeasureValueLabel.Text = Math.Round(Convert.ToSingle(dt.Rows[0]["MeasureValue"]), 2) + "m/s";
            }
       }
    }
}

言語仕様しか知識がないプログラマーは動作するプログラムを書こうとします。何の悪気もありません。

コンパイルエラーにならない事,実行時エラーにならない事のみが課題なのでこういったコードを書くことになります。

C#では結果的に何でもかけてしまいます。コンパイルではじかれたり,エラーになればだれもこんなコードは書かないのですが,書けてしまう以上,技術者が意識して悪いコードを書かないようにしなければなりません。

では,このコードはどこが悪いのでしょうか?悪い理由はいろいろあると思いますが,まず,SQLが重複するというのがあげられます。同じSQLを実行する画面が別にあった場合,その画面も同じSQLを書く必要が出ます。また接続先情報,タイムアウト時間,その他の作法に変更があった場合,すべてのデータベースアクセス部分に変更の必要があります。

データ取得後に値を加工していますが,小数点以下2桁で丸めたり,単位をつけたりするコードも共通化されません。こういったことが表面上の悪さとして挙げられます。

言語仕様の知識のみでのコーディングから,今度は共通化を意識したコーディングに変わります。これがプログラミングの第2段階だと思います。それでは共通化することで問題が解決されるか,良いコードになるかどうかを見ていきましょう。

まずSQLです。誰でも呼び出せる場所に移動しましょう。共通関数ということで「CommonFunc」という名前にしましょう。「Common」というフォルダを作成してそこに「CommonFunc」クラスを新規で追加します。

public class CommonFunc
{
    public static DataTable GetMeasureLatest()
    {
        string sql = @"select top 1 MeasureValue from Measures order by MeasureDate desc";
        var dt = new DataTable();
        using (var connection = new SqlConnection("XXXXXXここは接続先情報XXXXXX"))
        using (var adapter = new SqlDataAdapter(sql, connection))
        {
            connection.Open();
            adapter.Fill(dt);
            return dt;
        }
    }
}

publicなstatic関数にしてDataTableを返却します。
画面のコードは次のようになります。

private void DataButton_Click(object sender, EventArgs e)
{
    var dt = CommonFunc.GetMeasureLatest();
    if (dt.Rows.Count > 0)
    {
        this.MeasureValueLabel.Text = Math.Round(Convert.ToSingle(dt.Rows[0]["MeasureValue"]), 2) + "m/s";
    }
}

共通関数を呼び出し,DataTableに対する処理だけそのまま残します。データ取得後の加工処理も共通関数に移動します。計測値を取得して,小数点以下2桁で丸めて単位をつけて返却しています。

public static string MeasureValueString(float measureValue)
{
    return Math.Round(measureValue, 2) + "m/s";
}

画面コードも次のようにMeasureValueStringを呼び出します。

private void DataButton_Click(object sender, EventArgs e)
{
    var dt = CommonFunc.GetMeasureLatest();
    if (dt.Rows.Count > 0)
    {
        this.MeasureValueLabel.Text = CommonFunc.MeasureValueString(Convert.ToSingle(dt.Rows[0]["MeasureValue"]));
    }
}

これで共通化すべき部分はなくなりました。次に固定値の共通化としてconst値にしていきましょう。
MeasureValueStringの「2」や「m/s」を共通で使えるようにconst化します。

コンストの置き場所はCommonフォルダのCommonConstにしましょう。

public class CommonConst
{
    public const int MeasureValueDecimalPoint = 2;
    public const string MeasureValueUnitName = "m/s";
}

これで関数や固定値を共通化することができました。これでもう問題はないでしょうか?私は「言語仕様」を学んでたどり着く限界がここではないかと思っています。つまり,「共通化」と「コンスト化」までは誰もがたどり着きます。これ以上良いコードにするにはオブジェクト指向設計を学ばないと無理だと思います。

ここまでで表面上の悪いコードはなくなりました。しかし問題は多く残っています。何が問題でしょうか?

潜在的なBADポイント

潜在的なBADポイントとして2つ紹介します。まず1つ目は,知識の所在です。どこに知識があるかという問題です。ロジックには大きく分けて2種類あり,「使う側のコード」と「使われる側のコード」です。使う側のコードは「クライアントコード」や「アプリケーションロジック」等と呼んだりしますが,今回例では画面のロジックです。

使う側に知識があると,ドメインロジックが散らばるという問題があります。例えば画面ロジックで「計測値は小数点以下2桁で丸めて単位はm/s」というロジックがあると,知識があり,他の画面で計測値を表示するときは同じロジックが必要になり,ロジックが散らばります。

ただ今回はCommonFuncで共通化したので,クライアントコードに知識などないように見えます。

ただそれだけでは不十分です。確かにロジックは共通化されましたが,値とロジックがべつの場所にあるため,クライアントコードはそれを結びつける作業が必要です。

つまり「計測値」は「CommonFunc.MeasureValueString」を呼ばないといけないといつまでも記憶しておかないといけません。

一人の人間が作る分には覚えていられるかもしれませんが,チーム開発だとこういった箇所に不具合が良く出ます。CommonFunc.MeasureValueStringの存在を知らないメンバーが同じような関数を画面や,いろいろな箇所に書きだします。

また,直接画面に単位を書き込む人間も出てきます。

つまり値とロジックは一体化させて,使う側は,インテリセンスの中から選ぶだけの状態にしてあげるのが良いです。実装方法は後述します。

潜在的なBADポイント2つ目

潜在的なBADポイントの2つ目は「テスト容易性」です。テスト容易性とは「テストのしやすさ」という意味です。コードには「テストしやすいコード」と,「テストしにくいコード」があります。

テストしやすいコード

テストしやすいコードとは,例えば「どこにも依存していない関数」です。次のコードは関数内で完結しているため容易にテストコードを書くことができます。

public static int Add(int x, int y)
{
    return x + y;
}

関数内で他の関数を呼び出していたとしても,その関数がそこで完結していれば同様にテストは容易です。

public static int AddMultiply(int x, int y)
{
    return Multiply(x) + Multiply(y);
}

public static int Multiply(int value)
{
    return value * 2;
}

しかし,「アプリケーションの外部にアクセスしているコード」がある場合はテストしづらいコードになります。

「外部」とは,例えば「データベース」や「ファイル」等,物理的にアプリケーションの外側にあるものです。

それらにアクセスしていると,ファイルなどが存在していないとテストできないため,テストしづらいコードになります。

ファイルくらいならテスト用に置いておくこともできなくはないですが,外部機器などは物理的に存在しないとテストできないというのではテスト容易性は低いといえます。

銀行のATMなど,実機がないと動作確認ができないというのは良いプログラムとは言えません。

そういったコードはインターフェースを使用し,本番コードとテスト用のコードを切り分けてテスト容易性を高めることができます。

これに関しては後述の実践で解説します。

ここまでをふまえてBADコードを再確認しましょう。テストが容易にできるコードは「CommonFunc. MeasureValueString」だけです。

GetMeasureLatestはデータベースに依存しています。GetMeasureLatestがテストしづらい時点で,それを呼び出している「DataButton_Click」も同時にテストしづらいコードになります。

テストの観点からすれば,データベースアクセスコードが画面内にあろうと,他のクラスで共通関数になっていようと,それを呼び出している時点で,テスト容易性は同じです。

さらにテストしやすいコードとは外部から呼び出せないといけません。

画面コードの「DataButton_Click」のアクセスレベルはPrivateのためそもそもテストのために呼び出しができません。

これを呼び出せるようにしたとしても,テスト結果として「MeasureValueLabel.Text」の値を検証しないといけません。

画面のコントロールもPrivateになっていてテストできません。これをPublicにするとどこからでも値を変更される可能性がありよくありません。

そもそもテストのためにアクセスレベルを変更するというのはテストコードの書き方に誤りがあります。

この場合は画面をViewとViewModelに分割し,ViewModelをテストしViewModelのMeasureValueを画面のMeasureValue.Textにデータバインドすることでテストを可能とします。この具体的なやり方も後述の実践で詳しく解説します。