調べ物した結果

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

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されること」が前提。だったり、特定もメソッドの順番があったりで、
手を入れるのはそれなりに面倒な印象をうけた。
あとやはりコレクションクラスは作っておくにかぎるなぁ。ループのロジックが
そこら中にあふれ出すとかなりつらい。そんな感じ。