RevokeMsgPatcherのソースを眺める③~適当リファクタ~
今週のお相手 RevokeMsgPatcher
https://github.com/huiyadanli/RevokeMsgPatcher
例のごとく、なにをどうするものなのか知らないけど。
コードお借りします。
前回のおさらい。
・API(じゃないけど)絡むとテスト書くのめんどくせぇ。(二回目)
・コードメトリクスちょっと改善した。うれしい。
・ref & ref & refでまとめてセットするメソッドどうするべ・・・
ref & ref & refでまとめてセットするメソッドどうするべ
前回でセット部分をメソッドに切り出して、If分を簡略したのはいいけれど。
private void GetMatchingValues(App config, FileHexEditor editor, ref ModifyInfo sha1Before, ref ModifyInfo sha1After, ref ModifyInfo varsion)
の3ref。これがどうにもいやだなぁ。というのが今の状況。
値が変わるならvoidじゃなくて、なにか返してほしいし、
そこ以外は変わってほしくないんだよな。このひと固まりにとりあえず名前を付けようか。
入れ物クラスも嫌なんだけど。tupleはもっと嫌なんだよな。意味わからなくなるから。
public class MatchingValue { }
変数名にも使っているのでひとまずこのまま。メソッド移植する。
public class MatchingValue { private ModifyInfo _sha1Before { get; set; } private ModifyInfo _sha1After { get; set; } private ModifyInfo _varsion { get; set; } private MatchingValue() { } public static MatchingValue Get(App config, FileHexEditor editor) { var obj = new MatchingValue(); foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息 { if (modifyInfo.Name != editor.FileName) { continue; } if (editor.FileVersion == modifyInfo.Version) { obj._varsion = modifyInfo; } if (editor.FileSHA1 == modifyInfo.SHA1After) { obj._sha1After = modifyInfo; } else if (editor.FileSHA1 == modifyInfo.SHA1Before) { obj._sha1Before = modifyInfo; } } return obj; } }
おや・・・これはなかなか。。。よいのでは。呼び出し
はこうなる。
/// <summary> /// b.验证文件完整性,寻找对应的补丁信息 /// </summary> public void ValidateAndFindModifyInfo() { // 寻找对应文件版本与SHA1的修改信息 foreach (FileHexEditor editor in editors) // 多种文件 { // 通过SHA1和文件版本判断是否可以打补丁 根据不同结果返回不同的提示 MatchingValue matchingValue = MatchingValue.Get(config, editor); // 补丁前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}"); } } }
うむ。このままeditorの内容を変えちゃうメソッドもまとめて移動してしまおう。テストないけど。
public class MatchingValue { private ModifyInfo _sha1Before { get; set; } private ModifyInfo _sha1After { get; set; } private ModifyInfo _varsion { get; set; } private MatchingValue() { } public static MatchingValue Get(App config, FileHexEditor editor) { var obj = new MatchingValue(); foreach (ModifyInfo modifyInfo in config.FileModifyInfos[editor.FileName]) // 多个版本信息 { if (modifyInfo.Name != editor.FileName) { continue; } if (editor.FileVersion == modifyInfo.Version) { obj._varsion = modifyInfo; } if (editor.FileSHA1 == modifyInfo.SHA1After) { obj._sha1After = modifyInfo; } else if (editor.FileSHA1 == modifyInfo.SHA1Before) { obj._sha1Before = modifyInfo; } } return obj; } public FileHexEditor ReplaceModifyInfo(FileHexEditor editor) { // 补丁前SHA1匹配上,肯定是正确的dll if (IsBefore()) { editor.FileModifyInfo = _sha1Before; return editor; } // 补丁后SHA1匹配上,肯定已经打过补丁 if (IsAfter()) { throw new BusinessException("installed", $"你已经安装过此补丁,文件路径:{editor.FilePath}"); } // 全部不匹配,说明不支持 if (NotAll()) { throw new BusinessException("not_support", $"不支持此版本:{editor.FileVersion},文件路径:{editor.FilePath}"); } // SHA1不匹配,版本匹配,可能dll已经被其他补丁程序修改过 if (IsOnlyVersion()) { throw new BusinessException("maybe_modified", $"程序支持此版本:{editor.FileVersion}。但是文件校验不通过,请确认是否使用过其他补丁程序。文件路径:{editor.FilePath}"); } // 通らない return null; } private bool NotAll() { if (IsAfter()) { return false; } if (IsBefore()) { return false; } if (IsVersion()) { return false; } return true; } private bool IsOnlyVersion() { if (IsAfter()) { return false; } if (IsBefore()) { return false; } if (IsVersion()) { return true; } return false; } private bool IsVersion() { return _varsion != null; } private bool IsBefore() { return _sha1Before != null; } private bool IsAfter() { return _sha1After != null; } }
Is~のとこの命名はあんまり好きじゃない。ロジックによっていて設計のにおいがしない。
とりあえず諦めるか。
/// <summary> /// b.验证文件完整性,寻找对应的补丁信息 /// </summary> public void ValidateAndFindModifyInfo() { // 寻找对应文件版本与SHA1的修改信息 foreach (FileHexEditor editor in editors) // 多种文件 { // 通过SHA1和文件版本判断是否可以打补丁 根据不同结果返回不同的提示 MatchingValue matchingValue = MatchingValue.Get(config, editor); editor.FileModifyInfo = matchingValue.ReplaceModifyInfo(editor); } }
使ってるほうは死ぬほどシンプルになったぞ。めんどくさいものにふたをしたともいうが。そしてテストがしんだ。メソッドを消してしまったから・・・
private class FakeMatchingValue { private MatchingValue _value; public FakeMatchingValue(App config, FileHexEditor editor) { _value = MatchingValue.Get(config, editor); } public ModifyInfo before() { PrivateObject pobj = new PrivateObject(_value); return (ModifyInfo)pobj.GetProperty("_sha1Before"); } public ModifyInfo After() { PrivateObject pobj = new PrivateObject(_value); return (ModifyInfo)pobj.GetProperty("_sha1After"); } public ModifyInfo Version() { PrivateObject pobj = new PrivateObject(_value); return (ModifyInfo)pobj.GetProperty("_varsion"); } } [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" } }; // 該当なし。 var actual = new FakeMatchingValue(config, editor); editor.sha1 = "sha1"; // 全不一致でOK Assert.AreEqual(null, actual.before(), "全不一致"); Assert.AreEqual(null, actual.After(), "全不一致"); Assert.AreEqual(null, actual.Version(), "全不一致"); 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優先 actual = new FakeMatchingValue(config, editor); Assert.AreEqual(null, actual.before(), "afterとbeforeが一致している場合はafter優先"); Assert.AreEqual("sha1", actual.After().SHA1After, "afterとbeforeが一致している場合はafter優先"); Assert.AreEqual(null, actual.Version(), "afterとbeforeが一致している場合はafter優先"); // versionが設定されている場合はそちらも取得 pobj.SetField("version", "ver1"); actual = new FakeMatchingValue(config, editor); Assert.AreEqual(null, actual.before(), "versionが設定されている場合はそちらも取得"); Assert.AreEqual("sha1", actual.After().SHA1After, "versionが設定されている場合はそちらも取得"); Assert.AreEqual("ver1", actual.Version().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" } }; actual = new FakeMatchingValue(config, editor); Assert.AreEqual("sha1", actual.before().SHA1Before, "afterがアンマッチであればbeforeを取得"); Assert.AreEqual(null, actual.After(), "afterがアンマッチであればbeforeを取得"); Assert.AreEqual("ver1", actual.Version().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 } } }, }; actual = new FakeMatchingValue(config, editor); Assert.AreEqual(4, actual.before().Changes[0].Position); Assert.AreEqual(3, actual.After().Changes[0].Position); config.FileModifyInfos["test_dummy"].Reverse(); actual = new FakeMatchingValue(config, editor); Assert.AreEqual(2, actual.before().Changes[0].Position); Assert.AreEqual(1, actual.After().Changes[0].Position); }
テストも書き直した。リファクタを進めていくとどうにもテストが書きにくくなっていく。
外側からの変更ができなくなってくるからだけど。
メトリクスが改善されているかみてみる。
プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: AppModifier
保守容易性指数: 76
サイクロマティック複雑度: 30
継承の深さ: 1
クラス結合: 13
コード行: 53
おっけい。だいぶ改善した。追加したクラスもどうか
プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: MatchingValue
保守容易性指数: 76
サイクロマティック複雑度: 29
継承の深さ: 1
クラス結合: 8
コード行: 52
いいじゃないの。ぐっど!。というところで今日はおしまい。