ShareXのソースを眺める③~適当リファクタ~
https://couraeg.hatenablog.com/entry/2019/10/10/234323
の続き
Errorのクラス切り出し
どうもErrorクラスに切り出したほうがよさそうな気配がある。とりあえず切り出してみる
クラス内の参照を見る限り、ErrorsToString()をテストで保護してしまえば、
とりあえず切り出すことは可能に見える。
ErrorsToString
public string ErrorsToString() { if (IsError) { return string.Join(Environment.NewLine + Environment.NewLine, Errors.ToArray()); } return null; }
ますます Errorsをどうにかしたい欲がでるが。とりあえず保護だ。保護
isError の挙動が面倒
テストをするには、IsErrorがTrueであるように、オブジェクトをコントロールする必要がある。
これが面倒。IsErrorの修正にテストが引きずられるのもおかしいと思う。 Shim等の凶悪なMockがあればいいのだが。
private class FakeErrorsToString : UploadResult { private bool _isError; public FakeErrorsToString(bool isError) { _isError = isError; } public new bool IsError { get { return _isError; } } public string Invoke(List<string> errors) { if (IsError) { Errors = errors; return ErrorsToString(); } return null; } }
こーゆーFakeをかけないわけでもないが。もうこれ何のテストしてるのかわからない状態。
IsErrorの結果を受けて分岐することは残るには残るが。ふむ。素直にIsErrorの条件に合致するように、
objを生成してやろう。
幸いIsErrorのテストは書いてるから、条件は脳死で拾ってこれるはず。
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")); }
この辺。trueを結果にもっている条件のどれかをpickすればいいだろう。
[TestMethod] public void UploadResult_ErrorsToString_01() { // Setting IsError to false obj.Errors = null; obj.IsURLExpected = true; obj.URL = string.Empty; // if IsError is False then result is null. Assert.AreEqual(null, obj.ErrorsToString()); // Setting IsError to True List<string> errors = new List<string> { "dummy_error" }; obj.Errors = errors; obj.IsURLExpected = true; obj.URL = string.Empty; string expected = string.Join(Environment.NewLine + Environment.NewLine, errors.ToArray()); Assert.AreEqual(expected, obj.ErrorsToString()); }
かけた。これでいこう。
private Errors _errors { get; set; } public List<string> Errors { get { return _errors._value; } set { _errors._value = value; } }
互換を残すために、Errosは残したけど、これは余計にみにくいなぁ。
public string ErrorsToString() { if (IsError) { return _errors.ToString(); } return null; }
こっちは見通しがよくなったと思う。意図も伝わりやすくなったのではないか。
エラーがあるから、エラーを文字列でだす。
Errorsクラス
using System; using System.Collections.Generic; using System.Linq; namespace ShareX.UploadersLib { public class Errors { /// <remarks>keep compatibility public</remarks> public List<string> _value { get; set; } public Errors() { _value = new List<string>(); } public override string ToString() { return string.Join(Environment.NewLine + Environment.NewLine, _value.ToArray()); } public bool IsErrorsHasLeastOneError { get { if (_value == null) { return false; } if (!_value.Any()) { return false; } return true; } } } }
ファーストクラスにしたいが、影響が大きくてPublicで公開。
他の使われ方をみて、全部つぶしていくのがルートかなぁ。
テストも通ったのでこの辺りで・・・そろそろGitにPushするか。