ShareXのソースを眺める②～適当リファクタ～
つぎ。どうにも用途が怪しげなメソッド
public string ErrorsToString() { if (IsError) { return string.Join(Environment.NewLine + Environment.NewLine, Errors.ToArray()); } return null; }
Errorがあればエラーを出そうね。 ということだろう。
内容はともかく。Errors.ToArray ががポイント。
public List<string> Errors { get; set; } public bool IsURLExpected { get; set; } public bool IsError { get { return Errors != null && Errors.Count > 0 && (!IsURLExpected || string.IsNullOrEmpty(URL)); } }
うーん。ErrosToString()はこの位置でいいとしても（メソッド名は気になる）Errorsの内容はErrosクラスなりなんなりで抱えてほしいように思える。
厄介なことにIsUrlExpectedもそこそこ参照範囲がある。
とりあえず条件がつながっていてつらいから、いったんテストを書いてIsErrorを分解していこう
パターンが増えそうなので
実行用のInvokerメソッドをつくって、引数でどこが変わったかを見やすくしておく。
を使ってパターンを見やすくする。
/// <summary> /// A test when "erros" is null. When 'Erros' is null, 'return' is always false. /// </summary> [TestMethod] public void UploadResult_IsError_01() { Assert.AreEqual(false, Invoker_IsError(null, true, null)); Assert.AreEqual(false, Invoker_IsError(null, true, string.Empty)); Assert.AreEqual(false, Invoker_IsError(null, true, "dummy_url")); Assert.AreEqual(false, Invoker_IsError(null, false, null)); Assert.AreEqual(false, Invoker_IsError(null, false, string.Empty)); Assert.AreEqual(false, Invoker_IsError(null, false, "dummy_url")); } /// <summary> /// A test when "erros" is count == 0. When 'Erros' is count == 0, 'return' is always false. /// </summary> [TestMethod] public void UploadResult_IsError_02() { Assert.AreEqual(false, Invoker_IsError(new List<string>(), true, null)); Assert.AreEqual(false, Invoker_IsError(new List<string>(), true, string.Empty)); Assert.AreEqual(false, Invoker_IsError(new List<string>(), true, "dummy_url")); Assert.AreEqual(false, Invoker_IsError(new List<string>(), false, null)); Assert.AreEqual(false, Invoker_IsError(new List<string>(), false, string.Empty)); Assert.AreEqual(false, Invoker_IsError(new List<string>(), false, "dummy_url")); } /// <summary> /// A test when "erros" has least one error. /// </summary> [TestMethod] public void UploadResult_IsError_03() { // if url is null or empty, result is true. Assert.AreEqual(true, Invoker_IsError(new List<string> { "dummy_error"}, true, null)); Assert.AreEqual(true, Invoker_IsError(new List<string> { "dummy_error" }, true, string.Empty)); Assert.AreEqual(false, Invoker_IsError(new List<string> { "dummy_error" }, true, "dummy_url")); // if isURLExpected is false, result is true. Assert.AreEqual(true, Invoker_IsError(new List<string> { "dummy_error" }, false, null)); Assert.AreEqual(true, Invoker_IsError(new List<string> { "dummy_error" }, false, string.Empty)); Assert.AreEqual(true, Invoker_IsError(new List<string> { "dummy_error" }, false, "dummy_url")); } private bool Invoker_IsError(List<string> errors, bool isURLExpected, string url) { obj.Errors = errors; // set other parameters. obj.IsURLExpected = isURLExpected; obj.URL = url; return obj.IsError; }
これで網羅できた。安全に修正することができる。
とりあえず分解。早期リターンを積極的に活用していく。「＆＆」「｜｜」の結合子は少ないほうが頭が疲れない。
public bool IsError { get { // has not errors if (Errors == null) { return false; } if (!Errors.Any()) { return false; } // if (IsURLExpected) { return true; } if (string.IsNullOrEmpty(URL)) { return true; } return false; } }
// has not errors
のコメントはメソッドに切り出しておきたい。
public bool IsError { get { if (!IsErrorsHasLeastOneError) { return false; } if (IsURLExpected) { return true; } if (string.IsNullOrEmpty(URL)) { return true; } return false; } } private bool IsErrorsHasLeastOneError { get { if (Errors == null) { return false; } if (!Errors.Any()) { return false; } return true; } }
どうだろう。少なくともErrorsに一つのエラーがあって、
その他の要素で判断していることがわかりやすくなったのではないか。
この状態でもう一度テストを実行する。
テスト名: UploadResult_IsError_03
テストの完全名: Test.UploadResultTest.UploadResultTest.UploadResult_IsError_03
テスト ソース: D:\git_refact\ShareX\UnitTestProject1\UploadResultTest.cs : 行 49
テスト成果: 失敗
テスト継続期間: 0:00:00.092591
結果 のスタック トレース: 場所 Test.UploadResultTest.UploadResultTest.UploadResult_IsError_03() 場所 D:\git_refact\ShareX\UnitTestProject1\UploadResultTest.cs:行 54
結果 のメッセージ: Assert.AreEqual に失敗しました。
へへ。エラーになってしまいましたね。03のどこかがおかしいらしい。テストコードをみて確認してみる。
03のテストケースでexpectedにfalseが指定されているの3行目の一つだけ。どうやらこのケースでのなぜかTrueとして返却が返っている様子。
Assert.AreEqual(true, Invoker_IsError(new List
Assert.AreEqual(true, Invoker_IsError(new List
Assert.AreEqual(false, Invoker_IsError(new List
// if isURLExpected is false, result is true.
Assert.AreEqual(true, Invoker_IsError(new List
Assert.AreEqual(true, Invoker_IsError(new List
Assert.AreEqual(true, Invoker_IsError(new List
ここ、判定がひっくり返ってしまってますね。
if (IsURLExpected)
{
return true;
}
元々は
なので。 !IsURLExpected でTrueを返さないといけない。
&& (!IsURLExpected || string.IsNullOrEmpty(URL));
修正して再度テストを実行する
グループ名: Test.UploadResultTest
グループ化: Hierarchy
グループの完全名: Test.UploadResultTest
期間: 0:00:00.0098909
0 件のテストが失敗しました
0 件のテストがスキップされました
4 件のテストが合格しました
結果 1 の名前: UploadResult_IsError_01
結果 1 の成果: 成功
結果 1 の継続期間: 0:00:00.0084702
結果 1 のスタック トレース:
結果 1 のメッセージ:
結果 1 の標準出力:
結果 1 の標準エラー:
結果 2 の名前: UploadResult_IsError_02
結果 2 の成果: 成功
結果 2 の継続期間: 0:00:00.0001727
結果 2 のスタック トレース:
結果 2 のメッセージ:
結果 2 の標準出力:
結果 2 の標準エラー:
結果 3 の名前: UploadResult_IsError_03
結果 3 の成果: 成功
結果 3 の継続期間: 0:00:00.000309
結果 3 のスタック トレース:
結果 3 のメッセージ:
結果 3 の標準出力:
結果 3 の標準エラー:
結果 4 の名前: UploadResult_ToSummaryString_01
結果 4 の成果: 成功
結果 4 の継続期間: 0:00:00.000939
結果 4 のスタック トレース:
結果 4 のメッセージ:
結果 4 の標準出力:
結果 4 の標準エラー:
無事全件通ったということで。。。
やっぱりErrorsをそのままList
