タンバリンによるリファクタリング、またはハルクの鎮静方法

ほとんどのスタートアップはもともとひざまずいて作られていたことに誰もが同意すると思う。 その場合にのみ、適切なガイダンスと戦略的目標の理解により、成功した射撃の場合に、リソースの所有者は既存の製品のリファクタリングを決定できます。 まあ、もしこれがブルースバナーがハルクに変わる前に起こったら。 しかし、そのような瞬間が安全に見逃され、リソースが巨大な緑の不十分に制御された巨人である場合はどうでしょうか？ そのような状況で何をすべきか？



頭に浮かぶ最初の決定は、すべてを書き直して、脇の下にファウラーとGOFAのボリュームを保持することです。 しかし、この提案が真剣に受け止められる可能性は低いです。 2番目の解決策は既に受け入れられていますが、小さなステップでアレキサンダー大王を屈辱し、それでもゴーディアンノットを解き明かすことです。



この記事では、非常にわかりにくいソリューションをアプリケーションに変換し、明らかなレイヤーに分割し、単体テストでカバーした経験を共有したいと思います。 当然、この記事で示されている例はやや単純化されています。 私の意見では、彼らは基本的な概念を理解するのに十分です。 開始する前に、著者はコードの品質の最も重要な指標の1つである単体テストを検討していると言っておく価値があります。 また、ソリューションがWebformsを使用してASP.NETで記述されていることを明確にすることも価値があります。



それでは、ダンスを始めましょう。

最初の図は混乱しています。

対処しなければならなかった最初の問題は、htmlを生成するコードとDALコードの混同でした。 つまり、UserControl内のこのような落書きは頻繁に出会ったものです。

StringBuilder content = new StringBuilder(); var persons = Database.Persons.GetListBySchool(school); foreach(var person in persons) { content.AppendFormat(@”<p>{0}</p>”, person.Name); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

当然、この状況では、個人のリストを取得する実装の変更を夢見る理由はありません。 そして、私はこのコードを単体テストでカバーすることについては一般的に黙っています。 すぐに、UIとDALの間の接続をすばやく簡単に切断できる決定を下しました。

 public interface ICommand { void Execute(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 public abstract class Command<T> : ICommand { public T Result { get; protected set; } public abstract void Execute(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 public interface ICommandExecutor { T Run<T>(Command<T> command); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

データベースへのアクセスを必要とするビジネスロジックの各ユニットは、個別のチームになり、次のようになりました。

 public class GetListBySchool: Command<List<DbPerson>> { public DbSchool School { get; set; } public override void Execute() { this.Result = Database.Persons.GetListBySchool(this.School); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そして、これがICommandExecutorの実装です。

 public T Run<T>(Command<T> command) { command.Execute(); return command.Result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このソリューションを使用すると、ユーザーコントロール内のコードは次のようになります。

 private readonly ICommandExecutor executor; public SchoolView(ICommandExecutor executor) { if (executor == null) { throw new ArgumentNullException("executor"); } this.executor = executor; } public string GetPersonsHtmlList() { StringBuilder content = new StringBuilder(); var persons = this.executor.Run(new GetListBySchool { School = school }); foreach(var person in persons) { content.AppendFormat(@”<p>{0}</p>”, person.Name); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

すべてがうまくいくように思えますが、すべての直接データベース呼び出しをコマンドに置き換えた直後に、結果の更新されたコードもテストすることが不可能であることが明らかになりました。 そして、この理由は、コマンドによって返されるオブジェクトです。 DbPersonクラスには、パブリックセッターを持たない多くのロジックとプロパティが含まれていました。 最初のいくつかのプロパティは、 仮想としてマークされ、ロックされました。 しかし、3番目のプロパティはプライベートコンストラクターのみを持つタイプでした-これはつまずきのブロックになりました。 その瞬間、誰かが拳でテーブルをノックして、松葉杖なしでやることに決めたので、あなたはこの方向に固執する必要があると言いました。 仮想とマークされたプロパティはリセットされ、脳は別のオプションを求めて働きました。 実際、ソリューションは最初は表面にありましたが、退屈な作業が多く必要だったため、誰もそれを表明する勇気を持っていませんでした。 そして、解決策は次のとおりです： DTO 。 つまり、UserControls内では、DALオブジェクトのコピーであるオブジェクトを使用しますが、プログラムロジックはなく、パブリックゲッターとセッターを使用します。 このソリューションを可能にするために、コンバーターを作成しました。 便宜上、拡張メソッドが使用されました。



チームは次のようになり始めました。

 public class GetListBySchool: Command<List<Person>> { public long SchoolId { get; set; } public override void Execute() { var dbSchool = DataBase.Instance.GetSchoolById(this.SchoolId); this.Result = dbSchool.Network.Memberships .GetListBySchool(this.School) .Select(person => person.Convert()) .ToList(); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

GetPersonsHtmlListメソッドは完全にテスト可能になりましたが、別の問題が発生しました。UserControl内では、このようなメソッドだけではなく、多くのメソッドがあります。 また、以前にDbSchoolオブジェクトが一度だけロードされていた場合、個別のコマンドごとにロードされるようになりましたが、これはcomme il fautではありません。 この問題を解決するために、 IContextManagerにはメソッドが付属しています

 T GetFunctionResult<T>(Func<T> func) where T : class;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

HttpContext.Currentを使用して、実装を気にしませんでした 。 つまり、メソッドが初めて呼び出された場合、オブジェクトはコンテキストにスローされます。 メソッドが2回以上呼び出された場合、オブジェクトはコンテキストから取得されます。 IContextManagerの実装は、 CommandExecutorコンストラクターで指定され、すべてのコマンドに渡されます（コマンドの基本クラスに新しいプロパティが表示されます）。

 public T Run<T>(Command<T> command) { command.Context = this.context; command.Execute(); return command.Result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 public class GetListBySchool: Command<List<Person>> { public long SchoolId { get; set; } public override void Execute() { var dbSchool = this.Context.GetFunctionResult(() => DataBase.Instance.GetSchoolById(this.SchoolId)); this.Result = dbSchool.Network.Memberships .GetListBySchool(this.School) .Select(person => person.Convert()) .ToList(); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

その結果、比較的低コストで、UIレイヤーを分離することができました。 さらに重要なことは、この層を単体テストでカバーする機会があることです。 また、ビジネスロジックのレイヤーを強調する特定のメモも取得しました。 私の意見では、一連の個別のコマンドを持ち、それらをさまざまなサービスに結合することは、有能な構成をゼロから考えるよりも簡単です。

2番目の図は明らかです。

次のタスクは、Request.Paramsコレクションへのバインドを取り除くことでした。 しかし、ここでは誰も質問がありませんでした。解決策は次のとおりです。

 public interface IParametersReader { string GetRequestValue(string key); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 public class RequestParametersReader : IParametersReader { private readonly HttpContextBase context; public RequestParametersReader(HttpContextBase context) { if (context == null) { throw new ArgumentNullException("context"); } this.context = context; } public string GetRequestValue(string key) { return this.context.Request[key]; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

3番目の図は空気です。

通常、ページは1つのUserControlではなく、複数のUserControlで構成されます。 それらはすべて同じオブジェクトを必要とします。 最初に、次のソリューションが考案され、実装されました。子UserControlで、メソッドが作成されます

 void Bind(DbSchool school, DbPerson person) { this.school = school; this.person = person; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

次に、親UserControlからメソッドが呼び出されます。 すべてが素晴らしいようで、そのような決定には存在する権利があります。 ただし、コントロールのネストが2レベルを超えると、間違いを犯したり、目的の値を設定し忘れたりする可能性が大幅に増加することを忘れてはなりません。 その主なアイデアは、所定のソースからではなく、誰でも補充できるオブジェクトの一般的なコレクションからデータを取得することです。 このために2つのクラスが作成されました。

 public class InAttribute : Attribute { public string Name { get; set; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そして

 public class OutAttribute : Attribute { public string Name { get; set; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

空中に吊るす必要があるプロパティには、Out属性が付いています。 この空気から入力する必要があるプロパティは、類推によってIn属性によってマークされます。 System.Web.UI.UserControlを継承するクラスを作成し、次のコードを追加しました。

 protected override void OnLoad(EventArgs e) { //    var inProperties = this.GetType() .GetProperties() .Where(prop => prop.GetCustomAttributes(typeof(InAttribute), false).Any()); foreach (var property in inProperties) { var inAttribute = property.GetCustomAttributes(typeof(InAttribute), false).First() as InAttribute; if (inAttribute == null) { continue; } property.SetValue( this, this.context.Get(string.IsNullOrWhiteSpace(inAttribute.Name) ? property.Name : inAttribute.Name), null); } base.OnLoad(e); //    var outProperties = this.GetType() .GetProperties() .Where(prop => prop.GetCustomAttributes(typeof(OutAttribute), false).Any()); foreach (var property in outProperties) { var outAttribute = property.GetCustomAttributes( typeof(OutAttribute), false).First() as OutAttribute; if (outAttribute == null) { continue; } this.context.Add(string.IsNullOrWhiteSpace(outAttribute.Name) ? property.Name : outAttribute.Name, property.GetValue(this, null)); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エピローグ：

これは、 グリーンモンスターソフトウェア製品をリファクタリングする最初の段階でした。 私たちの仕事の結果は次のとおりです。



1.ユニットテストでUserControlを完全にカバーする機能。

2. UIレイヤーを他のすべてから明確に分離します。

3.ネストされたUserContol-s間の緊密なバインディングを取り除きます。

4.コードの読みやすさの改善。

5.道徳的苦痛の軽減と良心の浄化;）



記事の著者：Vitaly Lebedev、主任開発者Diary.ru