問題のあるデザインの兆候

デザインの良し悪しの概念は相対的です。 同時に、ほとんどの場合、その有効性、保守性、およびテスト容易性を保証するいくつかの確立されたプログラミング標準があります。 たとえば、オブジェクト指向言語では、これはカプセル化、継承、ポリモーフィズムの使用です。 場合によってはアプリケーションの設計にプラスの効果をもたらす一連の設計パターンがあります（場合によっては、状況に応じてすべてがマイナスになります）。 一方、反対の規範があり、それを順守すると、問題のある設計と呼ばれることがあります。 このような設計には通常、次の機能があります（同時に1つまたは複数）。

• 剛性（単純な変更は多くの場所に影響するため、コードを変更することは困難です）;
• 静止（コードを他のプログラムで使用できるモジュールに分割することは困難です）;
  
  
• 粘度（コードの開発および/またはテストはかなり難しい）;
  
  
• 不要な複雑さ（コードには未使用の機能があります）;
  
  
• 不要な再現性（コピー/貼り付け）;
  
  
• 可読性が低い（コードの機能を理解するのが難しく、維持するのが難しい）;
  
  
• 脆弱性（小さな変更でも機能を簡単に破ることができます）;
  
  

問題のある設計を避けたり、その使用の結果を予測したりするために、彼らは理解して区別できる必要があります。 これらの症状は、ロバートマーティンの著書「アジャイルの原則、パターン、C＃の実践」で説明されています。 ただし、このトピックに関する他のいくつかのレビュー記事（ちなみにそれほど多くはありません）のように、かなり簡単な説明が提供されており、原則としてコード例はありません。



これらの兆候のそれぞれに多かれ少なかれ焦点を当てることにより、この欠点を修正してみましょう。

剛性

すでに上で示したように、ハードコードは、たとえ細部であっても変更するのが難しい場合があります。 コードが頻繁に変更されない場合、またはまったく変更されない場合、これは問題になりません。 したがって、コードはまったく優れています。 しかし、彼が定期的に変更を必要とし、これを行うことが困難になると、彼は問題を抱えます。 それが機能する場合でも。



剛性の一般的なケースの1つは、抽象化（インターフェイス、基本クラスなど）を使用する代わりに、クラスタイプを明示的に示すことです。 以下はサンプルコードです。

class A { B _b;   public A() { _b = new B(); } public void Foo() {   // Do some custom logic. _b.DoSomething();   // Do some custom logic.   } } class B {  public void DoSomething() { // Do something } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここで、クラスAはクラスBに厳密に依存しています。つまり、将来Bの代わりに別のクラスを使用することにした場合、クラスAを変更する必要があるため、テストを繰り返します。 さらに、他の多くのクラスがBに厳密に依存している場合、状況は何度も複雑になる可能性があります。



この状況から抜け出す方法は、抽象化、つまりIComponentインターフェイスを導入し、クラスAのコンストラクター（または何らかの方法）を介してこのような依存関係を導入することです。この場合、クラスAは特定のクラスBに依存しなくなりますが、IComponentインターフェイスのみに依存します。 クラスBは、IComponentインターフェイスを実装する必要があります。

 interface IComponent { void DoSomething(); } class A { IComponent _component;  public A(IComponent component) {  _component = component; } void Foo() {   // Do some custom logic.  _component.DoSomething(); // Do some custom logic.   } } class B : IComponent { void DoSomething() {  // Do something } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

より具体的な例を挙げましょう。 情報をログに記録するクラスの特定のセット（ProductManagerとConsumer）があるとします。 彼らの仕事は、いくつかの製品をデータベースに保存し、それに応じて注文することです。 両方のクラスは関連するイベントを記録します。 まず、ファイルへのロギングが行われました。 このため、FileLoggerクラスが使用されました。 さらに、クラスは異なるモジュール（アセンブリ）に配置されていました。

 // Module 1 (Client) static void Main() { var product = new Product("milk"); var productManager = new ProductManager(); productManager.AddProduct(product); var consumer = new Consumer(); consumer.PurchaseProduct(product.Name); } // Module 2 (Business logic) public class ProductManager { private readonly FileLogger _logger = new FileLogger(); public void AddProduct(Product product) { // Add the product to the database. _logger.Log("The product is added."); } } public class Consumer { private readonly FileLogger _logger = new FileLogger(); public void PurchaseProduct(string product) { // Purchase the product. _logger.Log("The product is purchased."); } } public class Product { public string Name { get; private set; } public Product(string name) { Name = name; } } // Module 3 (Logger implementation) public class FileLogger { const string FileName = "log.txt"; public void Log(string message) { // Write the message to the file. } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

最初はファイルだけで十分であり、情報を収集して保存するためにデータベースやクラウドサービスなどの他のストレージにログを記録する必要がある場合、FileLoggerを使用してビジネスロジックモジュール（モジュール2）のすべてのクラスを変更する必要があります。 最終的に、これには注意が必要です。 この問題を解決するには、以下に示すように、ロガーを操作するための抽象的なインターフェースを導入できます。

 // Module 1 (Client) static void Main() { var logger = new FileLogger(); var product = new Product("milk"); var productManager = new ProductManager(logger); productManager.AddProduct(product); var consumer = new Consumer(logger); consumer.PurchaseProduct(product.Name); } // Module 2 (Business logic) class ProductManager { private readonly ILogger _logger; public ProductManager(ILogger logger) { _logger = logger; } public void AddProduct(Product product) { // Add the product to the database. _logger.Log("The product is added."); } } public class Consumer { private readonly ILogger _logger; public Consumer(ILogger logger) { _logger = logger; } public void PurchaseProduct(string product) { // Purchase the product. _logger.Log("The product is purchased."); } } public class Product { public string Name { get; private set; } public Product(string name) { Name = name; } } // Module 3 (interfaces) public interface ILogger { void Log(string message); } // Module 4 (Logger implementation) public class FileLogger : ILogger { const string FileName = "log.txt"; public virtual void Log(string message) { // Write the message to the file. } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この場合、ロガーのタイプを変更する場合、クライアントコード（メイン）を変更するだけで十分です。これにより、ロガーが初期化され、ProductManagerとConsumerのコンストラクターに配置されます。 したがって、ロガーのタイプに関する変更からビジネスロジッククラスを閉じました。これは実行する必要がありました。



使用されているクラスへの直接参照に加えて、コードを変更する際に問題を引き起こす他のオプションによって剛性を明示できます。 そのようなものは無限にあるかもしれませんが、別の例を挙げてみてください。 コンソールに幾何学図形の領域を表示するコードをいくつか用意してみましょう。

 static void Main() { var rectangle = new Rectangle() { W = 3, H = 5 }; var circle = new Circle() { R = 7 }; var shapes = new Shape[] { rectangle, circle }; ShapeHelper.ReportShapesSize(shapes); } class ShapeHelper { private static double GetShapeArea(Shape shape) { if (shape is Rectangle) { return ((Rectangle)shape).W * ((Rectangle)shape).H; } if (shape is Circle) { return 2 * Math.PI * ((Circle)shape).R * ((Circle)shape).R; } throw new InvalidOperationException("Not supported shape"); } public static void ReportShapesSize(Shape[] shapes) { foreach(Shape shape in shapes) { if (shape is Rectangle) { double area = GetShapeArea(shape); Console.WriteLine($"Rectangle's area is {area}"); } if (shape is Circle) { double area = GetShapeArea(shape); Console.WriteLine($"Circle's area is {area}"); } } } } public class Shape { } public class Rectangle : Shape { public double W { get; set; } public double H { get; set; } } public class Circle : Shape { public double R { get; set; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このコードは、新しい図形を追加するときに、ShapeHelperクラスのメソッドを変更する必要があることを示しています。 1つのオプションは、以下に示すように、レンダリングアルゴリズムを幾何学的図形（長方形と円）のクラス自体に転送することです。 これにより、各クラスの関連ロジックが分離されるため、コンソールに情報を出力する前にShapeHelperクラスの責任が絞り込まれます。

 static void Main() { var rectangle = new Rectangle() { W = 3, H = 5 }; var circle = new Circle() { R = 7 }; var shapes = new Shape[]() { rectangle, circle }; ShapeHelper.ReportShapesSize(shapes); } class ShapeHelper { public static void ReportShapesSize(Shape[] shapes) { foreach(Shape shape in shapes) { shape.Report(); } } } public abstract class Shape { public abstract void Report(); } public class Rectangle : Shape { public double W { get; set; } public double H { get; set; } public override void Report() { double area = W * H; Console.WriteLine($"Rectangle's area is {area}"); } } public class Circle : Shape { public double R { get; set; } public override void Report() { double area = 2 * Math.PI * R * R; Console.WriteLine($"Circle's area is {area}"); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

その結果、継承とポリモーフィズムを使用して新しいタイプのシェイプを追加するために、実際にShapeHelperクラスを閉じました。

静けさ

静止は、コードを再利用可能なモジュールに分割する複雑さで現れます。 その結果、プロジェクトは進化する能力を失い、結果として競争力を失う可能性があります。



例として、すべてのコードがアプリケーション（exe）の実行可能ファイルに実装され、ビジネスロジックを個別のモジュールまたはクラスに転送しないように設計されたデスクトッププログラムを考えます。 その後、しばらくして、プログラム開発者の前に次のビジネス要件が生じました。

• ユーザーインターフェイスを変更してWebアプリケーションにします。
• サードパーティのクライアントが独自のアプリケーションで使用できるように、プログラム機能をWebサービスのセットとして公開します。

この場合、すべてのプログラムコードが実行可能モジュールに配線されているため、これらの要件を満たすことは困難です。



次の図は、この機能の影響を受けない設計とは対照的に、固定設計の例を示しています。 それらは破線で区切られています。 図からわかるように、再利用可能なモジュール（ロジック）間のコードの配布、およびWebサービスレベルでの機能の公開により、さまざまなクライアントアプリケーション（アプリ）で使用できるようになり、間違いなく利点です。







静けさはモノリシックデザインとも呼ばれます。 それを石のブロックのように小さくて便利なコードに分割することは困難です。 この問題を回避する方法は？ 設計段階では、他のシステムでいずれかの機能を使用する可能性について考えることをお勧めします。 再利用される可能性のあるコードは、すぐに個別のモジュールとクラスに配置するのが最適です。

粘度

粘度は2つのタイプに分類できます。

• 粘度の発達;
• 環境の粘度。

開発の粘性は、選択したアプリケーション設計に従うのが比較的難しいことで示されます。 これは、プログラマーが満たす必要のある要件が多すぎるために、はるかに便利な開発パスがあるという事実による可能性があります。 環境の粘性は、アプリケーションの組み立て、展開、およびテストのプロセスの非効率性に現れます。



開発粘度の簡単な例として、他のコンポーネント（モジュール2およびモジュール3）で使用するために別のモジュール（モジュール1）に（設計により）配置する必要がある定数を使用した作業を挙げることができます。

 // Module 1 (Constants) static class Constants { public const decimal MaxSalary = 100M; public const int MaxNumberOfProducts = 100; } // Finance Module #using Module1 static class FinanceHelper { public static bool ApproveSalary(decimal salary) {  return salary <= Constants.MaxSalary; } } // Marketing Module #using Module1 class ProductManager { public void MakeOrder() {  int productsNumber = 0; while(productsNumber++ <= Constants.MaxNumberOfProducts) { // Purchase some product } } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

何らかの理由で、定数モジュールのアセンブリプロセスにかなりの時間がかかる場合、開発者がモジュールの終了を期待することは困難です。 さらに、定数モジュールには、ビジネスロジックの根本的に異なる部分（金融モジュールとマーケティングモジュール）に関連する非常に異種のエンティティが含まれているという事実に注意を払うことができます。 つまり、定数のモジュールは、互いに独立した理由で頻繁に変更される可能性があり、変更の同期プロセスという形で追加の問題を引き起こす可能性があります。

これはすべて、開発プロセスを遅くし、プログラマに負担をかける可能性があります。 粘性の低い設計のオプションは、ビジネスロジックの各モジュールに1つずつ、定数の個別のモジュールを作成するか、個別のモジュールを選択せず​​に定数を適切な場所に転送することです。



環境の粘性の例としては、リモートクライアント仮想マシンでのアプリケーションの開発とテストがあります。 このようなワークフローは、インターネット接続が遅いために耐えられない場合があり、開発者は記述されたコードの統合テストを体系的に無視できます。この機能を使用すると、クライアント側で問題が発生する可能性があります（バグ）。

不要な難易度

この場合、デザインには現在使用されていない機能があります（つまり、将来のためにですが、便利な場合はどうでしょうか）。 この事実は、プログラムのサポートと保守を複雑にし、開発とテストの時間を増加させる可能性があります。 たとえば、データベースからデータを読み込む必要があるプログラムを考えてみましょう。 このために、別のコンポーネントで使用される特別なDataManagerコンポーネントが作成されました。

 class DataManager { object[] GetData() {   // Retrieve and return data } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

開発者が、現在使用されていないデータベースにデータを書き込むための新しいメソッドをDataManagerに追加する場合（WriteData）、これは将来的にはほとんど使用されない可能性がありますが、これは不必要な複雑さの兆候です。



不要な複雑さのもう1つの例は、「あらゆる場合に対応する」インターフェイスです。 たとえば、文字列型のオブジェクトを受け入れる単一のProcessメソッドを持つインターフェイスを考えます。

 interface IProcessor { void Process(string message); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

タスクが厳密に定義された構造を持つ特定のタイプのメッセージを処理する場合、開発者にこの行を毎回特定のタイプのメッセージにデシリアライズさせるよりも、厳密に型指定されたインターフェイスを作成する方が簡単です。



別のポイントは、これがまったく必要ない場合の設計パターンに対する過度の熱意です。 結果として、これは粘度設計につながる可能性があります。



潜在的に未使用のコードを書くのに時間を無駄にするのはなぜですか？ QAは実際に公開され、サードパーティのクライアントが使用できるようになっているため、このようなコードをテストするために必要になる場合があります。 これもリリース時間を使い果たします。 可用性から得られる利益が開発とテストのコストを超えることを条件にのみ、将来の機能を設定する価値があります。

不要な再現性

おそらく開発者の大多数はこの機能に出会ったか、または出くわしました。これは、同じロジックまたは特定のコード全体の複数のコピーから成ります。 主な脅威は、変更を行う際のそのようなコードの脆弱性です。ある場所で何かを修正した後、別の場所でそれを行うのを忘れることがあります。 また、コードにこの機能がない場合に比べて、変更を行う際により多くの時間が必要です。



不必要な再現性は、開発者の過失の結果である可能性があります。また、コードを繰り返さないことは、それを行うよりもはるかに困難で危険な場合、設計の剛性/脆弱性の両方になります。 ただし、いずれにしても、再現性を許可しないでください。また、再利用可能なセクションを一般的なメソッドとクラスに転送することにより、コードを絶えず改善する必要があります。

読みにくい

この症状は、コードの読み取りと理解が困難であるという事実（およびその理由）に現れています。 可読性が低い理由は、コードの設計要件（構文、変数の命名、クラスなど）の違反、実装ロジックの混乱などです。



以下は、ブール変数を操作するメソッドを実装する読みにくいコードの例です。

 void Process_true_false(string trueorfalsevalue) { if (trueorfalsevalue.ToString().Length == 4) { // That means trueorfalsevalue is probably "true". Do something here. } else if (trueorfalsevalue.ToString().Length == 5) { // That means trueorfalsevalue is probably "false". Do something here. } else { throw new Exception("not true of false. that's not nice. return.") } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここでは、いくつかの問題を強調できます。 まず、メソッドと変数の名前は、一般に受け入れられている規則の対象ではありません。 第二に、メソッドの実装は最適ではありません。



おそらく、文字列ではなくブール値をすぐに受け入れる必要があります。 ただし、文字列を受け入れた場合でも、メソッドの先頭でブール値に変換し、文字列の長さを決定するメソッドを使用しない方が適切です。 第三に、除外テキスト（これは良くありません）がビジネススタイルと正式に一致していません。 そのようなテキストを読むと、コードは素人によって書かれたと感じるかもしれません（おそらく重要なポイントですが、それでも）。 メソッドは次のように書き換えることができます（文字列ではなくブール値を取る場合）。

 public void Process(bool value) { if (value) { // Do something. } else { // Do something. } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

次に、別のリファクタリングオプションを示します（まだ文字列を受け入れる必要がある場合）。

 public void Process(string value) { bool bValue = false; if (!bool.TryParse(value, out bValue)) { throw new ArgumentException($"The {value} is not boolean"); } if (bValue) { // Do something. } else { // Do something. } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

問題が発生し始めた場合、読みにくいコードに対してリファクタリングを実行する必要があります。 たとえば、そのメンテナンスと伝播が多数のバグの出現につながる場合。

もろさ

プログラムの脆弱性とは、変更があった場合の内訳の単純さを意味します。 内訳には、コンパイルエラーとランタイムエラーの2種類があります。 前者は剛性の裏側になる可能性があり、後者はクライアント側に既にあることが多いため、最も危険です。 ここで、それらは脆弱性の指標です。



インジケータは間違いなく相対的です。 誰かが非常に慎重にコードを修正し、失敗の可能性は低いです。 それどころか誰か-急いで、不注意で。 それでも、同じエグゼキューターでは、異なるコードが異なる数のエラーを生成する可能性があります。 おそらく、判読しにくいほど複雑になり、コンパイル段階ではなくプログラムの実行時間に依存するほど脆弱になります。



同時に、機能はしばしば変更されることさえなかったということを壊します。 さまざまなコンポーネントのロジック間の接続性が高いため、問題が発生する可能性があります。



特定の例を考えてみましょう。 ここで、特定のロール（roleIdパラメーターによって決定される）を使用したユーザー許可のロジックは、特定のリソース（resourceUri）にアクセスすることにより、静的メソッドによってアクセスされます。

 static void Main() { if (Helper.Authorize(1, "/pictures")) { Console.WriteLine("Authorized"); } } class Helper { public static bool Authorize(int roleId, string resourceUri) { if (roleId == 1 || roleId == 10) { if (resourceUri == "/pictures") { return true; } } if (roleId == 1 || roleId == 2 && resourceUri == "/admin") { return true; } return false; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ロジックがかなり「毛深い」ことに気付くかもしれません。 明らかに、新しい役割とリソースを追加すると、簡単に壊れてしまいます。 その結果、一部のロールが特定のリソースへのアクセスを取得または喪失する場合があります。 以下に示すように、リソース識別子とサポートされているロールのリストを保存するResourceクラスを作成すると、脆弱性が軽減されます。

 static void Main() { var picturesResource = new Resource() { Uri = "/pictures" }; picturesResource.AddRole(1); if (picturesResource.IsAvailable(1)) { Console.WriteLine("Authorized"); } } class Resource { private List<int> _roles = new List<int>(); public string Uri { get; set; } public void AddRole(int roleId) { _roles.Add(roleId); } public void RemoveRole(int roleId) { _roles.Remove(roleId); } public bool IsAvailable(int roleId) { return _roles.Contains(roleId); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この場合、新しいリソースとロールを追加するために、承認ロジックコードの変更はまったく必要ありません。つまり、本質的に破壊するものはありません。



実行時エラーをキャッチするのに役立つもの 手動、自動、ユニットテスト。 テストプロセスを適切に編成すればするほど、脆弱なコードがクライアント側に現れるという保護が強化されます。



脆弱性は、多くの場合、剛性、読みやすさ、不必要な再現性など、デザインの悪さの裏返しです。

おわりに

問題設計の主な機能を強調して説明しようとしました。 それらのいくつかは相互接続されています。 設計上の問題が常に困難につながるとは限らず、発生の可能性を示すだけであることを明確に理解する必要があります。 これらの兆候のトレースが少ないほど、この確率は低くなります。