とある日のコード改善メモ（共通処理のくくり出し）

とある日のコード改善メモ.md

状況の解説

以下のようなユーザからの入力 (JSON payload) があったとき...

{
  "parameterA": "Enable",
  "parameterB": "Disable",
  ...
}

0. 入力が省略されていたパラメタがあった (nullだった) 場合は、そのパラメタをそのデフォルト値で補完する。

   • 今回は、変数defaultValuesに各パラメタのデフォルト値があらかじめ格納されているものとする。
     • 例: parameterAのデフォルト値はdefaultValues.ParameterAに格納されている。

1. パラメタが入力されていたら、それが妥当な値かどうかを検証してから、それを格納する。

   • 「妥当な値かどうかの検証」は、メソッドValidateParameterByType()で行うものとする。
     • ここで、このメソッドを使用する際は、各パラメタに応じた Type を指定するものとする。
       • 例: parameterAには、ParameterTypeEnum.TYPE_Aの Type を指定する。

改善前のコード

void GetValidatedParameters()
{
    if (payload.parameterA == null)
    {
        // filled with its default value if the value of the payload is empty
        this.ValidatedParameterA = defaultValues.ParameterA;
    }
    else
    {
        // filled with validated value if the value of the payload is not empty
        this.ValidatedParameterA
            = ValidateParameterByType(ParameterTypeEnum.TYPE_A, payload.parameterA);
    }
    
    if (payload.parameterB == null)
    {
        this.ValidatedParameterB = defaultValues.ParameterB;
    }
    else
    {
        this.ValidatedParameterB
            = ValidateParameterByType(ParameterTypeEnum.TYPE_B, payload.parameterB);
    }
    /* And so on... */
}

改善点

0. 各パラメタ間で共通処理っぽい部分があるので、GetValidatedParameterCommon()としてくくり出してみる。
1. さらに、if/else 文が長く感じるので、三項演算子にしてみる。

改善後のコード

void GetValidatedParameters()
{
    this.ValidatedParameterA = GetValidatedParameterCommon(payload.parameterA,
                                                           defaultValues.ParameterA,
                                                           ParameterTypeEnum.TYPE_A);
    this.ValidatedParameterB = GetValidatedParameterCommon(payload.parameterB,
                                                           defaultValues.ParameterB,
                                                           ParameterTypeEnum.TYPE_B);
    /* And so on... */
}

private string GetValidatedParameterCommon(string payloadValue,
                                           string defaultValue,
                                           ParameterTypeEnum parameterType)
{
    return payloadValue == null ? defaultValue
                                : ValidateParameterByType(parameterType, payloadValue);
}

改善後のコードの良い点

• GetValidatedParameters() の部分で言うと...
  • コードが短くなった。
    • 書く量が減るので、コーディングミスも減りそう。
    • 処理がまとまっており、一覧性が高いので、コーディングミスにも気づきやすい。
• GetValidatedParameterCommon() の部分で言うと...
  • ロジックがシンプルになった。
    • 「ここはパラメタＡについての箇所だから、パラメタＡのデフォルト値を使って...」などと頭を使わずに読む／書くことができるようになった。
  • 三項演算子を使うかどうかは議論の余地があるかもしれない。
    • このレベルであれば if/else 文でもいいのかもしれない。