RevokeMsgPatcherのソースを眺める④~適当リファクタ~
今週のお相手 RevokeMsgPatcher
https://github.com/huiyadanli/RevokeMsgPatcher
例のごとく、なにをどうするものなのか知らないけど。
コードお借りします。
前回のおさらい。
・コードメトリクスだいぶ改善した。うれしい。
・ref & ref & refでまとめてセットするメソッドはとりあえずクラスにきって、
メソッドあつめたらええ。
さて
コードメトリクスもだいぶ改善されてきた。
もともとそこまでひどい数値ではなかったけど。
残すメソッドもわずかで、つぎのメソッドを退治したらこのツールともいったんお別れかな。
Patch() メソッド
/// <summary> /// c.根据补丁信息,安装补丁 /// </summary> /// <returns></returns> public bool Patch() { // 首先验证文件修改器是否没问题 foreach (FileHexEditor editor in editors) { if (editor.FileModifyInfo == null) { throw new Exception("补丁安装失败,原因:文件修改器初始化失败!"); } } // 再备份所有文件 foreach (FileHexEditor editor in editors) { editor.Backup(); } // 打补丁! List<FileHexEditor> done = new List<FileHexEditor>(); // 已经打上补丁的 try { foreach (FileHexEditor editor in editors) { bool success = editor.Patch(); if (!success) { editor.Restore(); } done.Add(editor); } } catch (Exception ex) { // 恢复所有已经打上补丁的文件 foreach (FileHexEditor editor in done) { editor.Restore(); } throw ex; } return true; }
そんなに入り組んではいないけど、またなんかテスト書きにくそうな
雰囲気があっていやだなぁ。
FileHexEditorとか、その抱えているリストでやっちゃいなよ。というよなメソッドに
見えなくもない。 というか移動でよいのでは。
ひとまず、どうにもEditorsクラスの必要性を感じるので作ってみる。
Editors
namespace RevokeMsgPatcher.Modifier { public class Editors : List<FileHexEditor> { private Editors() { } public static Editors Get() { return new Editors(); } } }
本当はListを継承するのもいやなんだけど、色んな所で使われていて、
まとめて直す範囲が大きくなりすぎるので、元の機能はのこしつつ。といった形で作ってい。
もとのメソッドはすごくシンプルになったぞ
/// <summary> /// c.根据补丁信息,安装补丁 /// </summary> /// <returns></returns> public bool Patch() { editors.CheckNotAllNull(); editors.Backup(); editors.Done(); return true; }
順番が決まっているので、もう一声いこう。まさしくくさいものに
蓋スタイル
/// <summary> /// c.根据补丁信息,安装补丁 /// </summary> /// <returns></returns> public bool Patch() { editors.Patch(); return true; }
using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Threading.Tasks; namespace RevokeMsgPatcher.Modifier { public class Editors : List<FileHexEditor> { private List<FileHexEditor> _values { get { return this; } } private Editors() { } public static Editors Get() { return new Editors(); } public void Patch() { CheckNotAllNull(); Backup(); Done(); } private void Done() { // 打补丁! Editors done = Editors.Get(); // 已经打上补丁的 try { foreach (FileHexEditor editor in _values) { bool success = editor.Patch(); if (!success) { editor.Restore(); } done.Add(editor); } } catch (Exception ex) { done.Restore(); throw ex; } } private void Restore() { // 恢复所有已经打上补丁的文件 foreach (FileHexEditor editor in _values) { editor.Restore(); } } private void CheckNotAllNull() { // 首先验证文件修改器是否没问题 if (_values.Any(v => v.FileModifyInfo == null)) { throw new Exception("补丁安装失败,原因:文件修改器初始化失败!"); } } private void Backup() { // 再备份所有文件 foreach (FileHexEditor editor in _values) { editor.Backup(); } } } }
LInqをまじえつつ。可読性を意識して書いたらこんな感じになった。
Linqも場所間違えなければめちゃくちゃ有能なんだけど、多用すると途端に見づらくなるよな。
内部のDoneのRestoreの処理はすごく嫌だ。とても嫌だけどそのまま置いといた。ノータッチ。
最後にメトリクスを計算してみる。
プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: AppModifier
保守容易性指数: 81
サイクロマティック複雑度: 23
継承の深さ: 1
クラス結合: 14
コード行: 38
プロジェクト: RevokeMsgPatcher
構成: Debug
スコープ: 型
アセンブリ: D:\git_refact\RevokeMsgPatcher\RevokeMsgPatcher\bin\Debug\RevokeMsgPatcher.exe
名前空間: RevokeMsgPatcher.Modifier
型: Editors
保守容易性指数: 77
サイクロマティック複雑度: 16
継承の深さ: 2
クラス結合: 8
コード行: 27
もともとの保守容易性指数は「69」だったので、ずいぶん改善したんじゃなかろうか。
とりあえず終わり。
1週間ごとに適当にソースを眺めてテスト書いたり、リファクタするだけで
結構発見があるものだな。
いままでいかにそのあたりを無視して進めていたのか。ということでもあるだろう。
今回のソースはもともとかなり見やすかったけど、
やっぱり「throwされること」が前提。だったり、特定もメソッドの順番があったりで、
手を入れるのはそれなりに面倒な印象をうけた。
あとやはりコレクションクラスは作っておくにかぎるなぁ。ループのロジックが
そこら中にあふれ出すとかなりつらい。そんな感じ。