調べ物した結果

現役SEが仕事と直接関係ないことを調べた結果とか感想とか

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するか。