RevokeMsgPatcherのソースを眺める②～適当リファクタ～

今週のお相手　RevokeMsgPatcher

https://github.com/huiyadanli/RevokeMsgPatcher
例のごとく、なにをどうするものなのか知らないけど。
コードお借りします。

前回のおさらい。

・API（じゃないけど）絡むとテスト書くのめんどくせぇ。
・コードメトリクス全然改善しとらんやんけ。

プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: AppModifier
保守容易性指数: 69
サイクロマティック複雑度: 43
継承の深さ: 1
クラス結合: 14
コード行: 70

もともときれいなコードになっているので、結構数直さないと
数値改善しないだろうなぁ。と思っている。ゴリゴリやっていこう。

public void ValidateAndFindModifyInfo()　をリファクタする

        public void ValidateAndFindModifyInfo()
        {
            // 寻找对应文件版本与SHA1的修改信息
            foreach (FileHexEditor editor in editors) // 多种文件
            {
                // 通过SHA1和文件版本判断是否可以打补丁 根据不同结果返回不同的提示
                ModifyInfo matchingSHA1Before = null, matchingSHA1After = null, matchingVersion = null;
                foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息
                {
                    if (modifyInfo.Name == editor.FileName) // 保险用的无用判断
                    {
                        if (editor.FileSHA1 == modifyInfo.SHA1After)
                        {
                            matchingSHA1After = modifyInfo;
                        }
                        else if (editor.FileSHA1 == modifyInfo.SHA1Before)
                        {
                            matchingSHA1Before = modifyInfo;
                        }

                        if (editor.FileVersion == modifyInfo.Version)
                        {
                            matchingVersion = modifyInfo;
                        }
                    }
                }

                // 补丁前SHA1匹配上，肯定是正确的dll
                if (matchingSHA1Before != null)
                {
                    editor.FileModifyInfo = matchingSHA1Before;
                    continue;
                }
                // 补丁后SHA1匹配上，肯定已经打过补丁
                if (matchingSHA1After != null)
                {
                    throw new BusinessException("installed", $"你已经安装过此补丁，文件路径：{editor.FilePath}");
                }
                // 全部不匹配，说明不支持
                if (matchingSHA1Before == null && matchingSHA1After == null && matchingVersion == null)
                {
                    throw new BusinessException("not_support", $"不支持此版本：{editor.FileVersion}，文件路径：{editor.FilePath}");
                }
                // SHA1不匹配，版本匹配，可能dll已经被其他补丁程序修改过
                if ((matchingSHA1Before == null && matchingSHA1After == null) && matchingVersion != null)
                {
                    throw new BusinessException("maybe_modified", $"程序支持此版本：{editor.FileVersion}。但是文件校验不通过，请确认是否使用过其他补丁程序。文件路径：{editor.FilePath}");
                }
            }
        }

なかなか骨が折れそうなうえに、voidでテストしにくそうだけど、これが消えればそれなりの改善が見込めると思う。
地道に分解していく。
とりあえず後半のifの塊は無視して、前半のネストされているforeachに着目しよう。

                // 通过SHA1和文件版本判断是否可以打补丁 根据不同结果返回不同的提示
                ModifyInfo matchingSHA1Before = null, matchingSHA1After = null, matchingVersion = null;
                foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息
                {
                    if (modifyInfo.Name == editor.FileName) // 保险用的无用判断
                    {
                        if (editor.FileSHA1 == modifyInfo.SHA1After)
                        {
                            matchingSHA1After = modifyInfo;
                        }
                        else if (editor.FileSHA1 == modifyInfo.SHA1Before)
                        {
                            matchingSHA1Before = modifyInfo;
                        }

                        if (editor.FileVersion == modifyInfo.Version)
                        {
                            matchingVersion = modifyInfo;
                        }
                    }
                }

大体・・・よめたかなぁ。
・editor.fileNameでinfosから取得
・modifyInfo.Name==editor.FileName判定（これなんか気持ち悪いんだよなぁ）
・あとは比較してセットして。
あーテスト書くのしんどいなぁ。とりあえず塊のままメソッドに切り出すか。このままだとテスト書けない。

        public void GetMatchingValues(App config, FileHexEditor editor,  ref ModifyInfo sha1Before, ref ModifyInfo sha1After, ref ModifyInfo varsion)
        {
            foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息
            {
                if (modifyInfo.Name == editor.FileName) // 保险用的无用判断
                {
                    if (editor.FileSHA1 == modifyInfo.SHA1After)
                    {
                        matchingSHA1After = modifyInfo;
                    }
                    else if (editor.FileSHA1 == modifyInfo.SHA1Before)
                    {
                        matchingSHA1Before = modifyInfo;
                    }

                    if (editor.FileVersion == modifyInfo.Version)
                    {
                        matchingVersion = modifyInfo;
                    }
                }
            }
        }

とりあえずそとっつらだけ。変数名長かったので、エラーになったとこだえ辻褄合わせる
順番が・・・おいといてテストかこうか。

        public void GetMatchingValues(App config, FileHexEditor editor,  ref ModifyInfo sha1Before, ref ModifyInfo sha1After, ref ModifyInfo varsion)
        {
            foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息
            {
                if (modifyInfo.Name == editor.FileName) // 保险用的无用判断
                {
                    if (editor.FileSHA1 == modifyInfo.SHA1After)
                    {
                        sha1After = modifyInfo;
                    }
                    else if (editor.FileSHA1 == modifyInfo.SHA1Before)
                    {
                        sha1Before = modifyInfo;
                    }

                    if (editor.FileVersion == modifyInfo.Version)
                    {
                        varsion = modifyInfo;
                    }
                }
            }
        }

適当にかいたら内部的にファイルアクセスしてておちる。
具体的にFileVersionの取得処理。まいったなぁ。

        private string version;
        public string FileVersion
        {
            get
            {
                if (version == null)
                {
                    version = FileUtil.GetFileVersion(FilePath);
                }
                return version;
            }
        }

そろそろMoqなりを使わないと厳しくなってきている。
privateobjecteでセットすれば切り抜けれるかな。

            FileHexEditor editor = new FileHexEditor(string.Empty, new TargetInfo() { RelativePath = string.Empty, Name = "test_dummy" });
            PrivateObject pobj = new PrivateObject(editor);
            pobj.SetField("version", "ver1");

これで行けた。恐ろしい機能だ。

 [TestMethod]
        public void Test_GetMatchingValues()
        {

            FileHexEditor editor = new FileHexEditor(string.Empty, new TargetInfo() { RelativePath = string.Empty, Name = "test_dummy" });
            PrivateObject pobj = new PrivateObject(editor);
            pobj.SetField("version", string.Empty);

            App config = new App() { FileModifyInfos = new Dictionary<string, List<ModifyInfo>>() };
            config.FileModifyInfos["test_dummy"] = new List<ModifyInfo>() { new ModifyInfo() { Name = "else", SHA1After="sha1", SHA1Before="sha1", Version ="ver1"} };
            ModifyInfo matchingSHA1Before = null, matchingSHA1After = null, matchingVersion = null;

            // 該当なし。
            FakeAppModifier obj = FakeAppModifier.GetMatchingValues();
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            editor.sha1 = "sha1";

            // 全不一致でOK
            Assert.AreEqual(null, matchingSHA1Before, "全不一致");
            Assert.AreEqual(null, matchingSHA1After, "全不一致");
            Assert.AreEqual(null, matchingVersion, "全不一致");

            config = new App() { FileModifyInfos = new Dictionary<string, List<ModifyInfo>>() };
            config.FileModifyInfos["test_dummy"] = new List<ModifyInfo>() { new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1", SHA1Before = "sha1", Version = "ver1" } };

            // afterとbeforeが一致している場合はafter優先
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            Assert.AreEqual(null, matchingSHA1Before, "afterとbeforeが一致している場合はafter優先");
            Assert.IsNotNull(matchingSHA1After, "afterとbeforeが一致している場合はafter優先");
            Assert.AreEqual("sha1", matchingSHA1After.SHA1After, "afterとbeforeが一致している場合はafter優先");
            Assert.AreEqual(null, matchingVersion, "afterとbeforeが一致している場合はafter優先");

            // versionが設定されている場合はそちらも取得
            pobj.SetField("version", "ver1");
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            Assert.AreEqual(null, matchingSHA1Before,"versionが設定されている場合はそちらも取得");
            Assert.IsNotNull(matchingSHA1After, "versionが設定されている場合はそちらも取得");
            Assert.AreEqual("sha1", matchingSHA1After.SHA1After, "versionが設定されている場合はそちらも取得");
            Assert.AreEqual("ver1", matchingVersion.Version, "versionが設定されている場合はそちらも取得");

            // afterがアンマッチであればbeforeを取得
            config = new App() { FileModifyInfos = new Dictionary<string, List<ModifyInfo>>() };
            config.FileModifyInfos["test_dummy"] = new List<ModifyInfo>() { new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1_dum", SHA1Before = "sha1", Version = "ver1" } };
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            Assert.IsNotNull(matchingSHA1Before, "afterがアンマッチであればbeforeを取得");
            Assert.AreEqual("sha1", matchingSHA1Before.SHA1Before, "afterがアンマッチであればbeforeを取得");
            Assert.AreEqual(null, matchingSHA1After, "afterがアンマッチであればbeforeを取得");
            Assert.AreEqual("ver1", matchingVersion.Version, "afterがアンマッチであればbeforeを取得");

            // 複数行。マッチする中で最終レコードが採用される
            config = new App() { FileModifyInfos = new Dictionary<string, List<ModifyInfo>>() };
            config.FileModifyInfos["test_dummy"] = new List<ModifyInfo>()
            {
                // 内容だと判断しづらいので、Changesを使って判定する
                new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1", SHA1Before = "sha1_dum", Version = "ver1", Changes = new List<Change>(){ new Change() { Position = 1 } } },
                new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1_dum", SHA1Before = "sha1", Version = "ver1", Changes = new List<Change>(){ new Change() { Position = 2 } } },
                new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1", SHA1Before = "sha1_dum", Version = "ver1", Changes = new List<Change>(){ new Change() { Position = 3 } } },
                new ModifyInfo() { Name = "test_dummy", SHA1After = "sha1_dum", SHA1Before = "sha1", Version = "ver1", Changes = new List<Change>(){ new Change() { Position = 4 } } },
            };
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            Assert.AreEqual(4, matchingSHA1Before.Changes[0].Position);
            Assert.AreEqual(3, matchingSHA1After.Changes[0].Position);

            config.FileModifyInfos["test_dummy"].Reverse();
            obj.Invoke_GetMatchingValues(config, editor, ref matchingSHA1Before, ref matchingSHA1After, ref matchingVersion);
            Assert.AreEqual(2, matchingSHA1Before.Changes[0].Position);
            Assert.AreEqual(1, matchingSHA1After.Changes[0].Position);
        }

とりあえずかいたが。恐ろしく見にくい。とりあえず既存の動作は保証できたので余計なネストを破壊しよう。

        private void GetMatchingValues(App config, FileHexEditor editor,  ref ModifyInfo sha1Before, ref ModifyInfo sha1After, ref ModifyInfo varsion)
        {
            foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息
            {
                if (modifyInfo.Name != editor.FileName)
                {
                    continue;
                }

                if (editor.FileVersion == modifyInfo.Version)
                {
                    varsion = modifyInfo;
                }

                if (editor.FileSHA1 == modifyInfo.SHA1After)
                {
                    sha1After = modifyInfo;
                }
                else if (editor.FileSHA1 == modifyInfo.SHA1Before)
                {
                    sha1Before = modifyInfo;
                }
            }
        }

beforeafterのところが悩ましい。もう少しシンプルに書けないかなぁ。
最初のForeachもいやだなぁ。FileModifyInfosが処理してくれたらいいのに。
疲れたのでいったんこの辺にしておこう。続きはまた今度だ。

最後にメトリックスを計測する。

プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: AppModifier
保守容易性指数: 71
サイクロマティック複雑度: 44
継承の深さ: 1
クラス結合: 14
コード行: 73

少し改善した。やったぜ。

プロジェクト: Test.RevokeMsgPatcher
構成: Debug
スコープ: プロジェクト
アセンブリ: D:\git_refact\RevokeMsgPatcher\Test.RevokeMsgPatcher\bin\Debug\Test.RevokeMsgPatcher.dll
保守容易性指数: 68
サイクロマティック複雑度: 20
継承の深さ: 2
クラス結合: 20
コード行: 104

テストの保守性が最悪である。これはこれでダメだよなぁ。