2008-10-09 16 views
8

Chạy FxCop trên mã của tôi, tôi nhận được cảnh báo này:Khớp nối quá cao - cách thiết kế lớp này tốt hơn?

Microsoft.Maintainability: 'FooBar.ctor được kết hợp với 99 loại khác nhau từ 9 không gian tên khác nhau. Viết lại hoặc cấu trúc lại phương pháp để giảm khớp nối lớp, hoặc xem xét di chuyển phương thức sang một số của các loại khác là chặt chẽ cùng với. Một khớp nối lớp trên 40 cho thấy khả năng bảo trì kém, một khớp nối lớp giữa 40 và 30 cho thấy khả năng bảo trì vừa phải, và khớp nối lớp bên dưới 30 cho thấy khả năng bảo trì tốt.

Lớp học của tôi là vùng đích cho tất cả thư từ máy chủ. Máy chủ có thể gửi cho chúng tôi thông báo về các loại EventArgs khác nhau:

public FooBar() 
{ 
    var messageHandlers = new Dictionary<Type, Action<EventArgs>>(); 
    messageHandlers.Add(typeof(YouHaveBeenLoggedOutEventArgs), HandleSignOut); 
    messageHandlers.Add(typeof(TestConnectionEventArgs), HandleConnectionTest); 
    // ... etc for 90 other types 
} 

Phương thức "HandleSignOut" và "HandleConnectionTest" có ít mã trong đó; họ thường chuyển công việc sang một hàm trong lớp khác.

Làm cách nào để làm cho lớp này tốt hơn với khớp nối thấp hơn?

+0

Tôi thích các câu trả lời ở đây nhưng có thể một nơi nào đó như RefactorMyCode.com có ​​thể là một nơi tốt hơn cho nó. Việc mọi người đăng mã trong phản hồi của họ trên trang web đó dễ dàng hơn một chút. –

+0

Ok, tôi đã đăng ví dụ, kiểm tra xem nó ra –

Trả lời

15

Có các lớp học mà làm đăng ký công việc cho các sự kiện mà họ quan tâm ... một mẫu event broker.

class EventBroker { 
    private Dictionary<Type, Action<EventArgs>> messageHandlers; 

    void Register<T>(Action<EventArgs> subscriber) where T:EventArgs { 
     // may have to combine delegates if more than 1 listener 
     messageHandlers[typeof(T)] = subscriber; 
    } 

    void Send<T>(T e) where T:EventArgs { 
     var d = messageHandlers[typeof(T)]; 
     if (d != null) { 
     d(e); 
     } 
    } 
} 
+0

Chính xác những gì tôi vừa nói đến. Tôi sẽ bầu bạn vì ví dụ. – NotMe

+0

Thú vị! Cảm ơn bạn đã trả lời, chúng tôi sẽ xem xét điều này. Tôi nghĩ rằng tôi sẽ chấp nhận của bạn như là câu trả lời cho đến khi tôi nhìn thấy một tốt hơn. –

+0

Thú vị! Tôi chạy vào cùng một vấn đề. Có đủ nhanh không? Ý tôi là, tôi có thể sử dụng nó trong trò chơi với vòng lặp cập nhật không? – Vinz243

0

Có lẽ thay vì có một lớp khác nhau cho mỗi thư, hãy sử dụng cờ xác định thư.

Điều đó sẽ làm giảm đáng kể số lượng thư bạn có và tăng khả năng bảo trì. Tôi đoán là hầu hết các lớp thông báo đều có chênh lệch về 0.

Thật khó để chọn một cách bổ sung để tấn công điều này vì phần còn lại của kiến ​​trúc là không xác định (đối với tôi).

Nếu bạn nhìn vào Windows, ví dụ, nó không biết cách xử lý từng thư có thể bị ném về. Thay vào đó, các trình xử lý thông báo bên dưới đăng ký các hàm gọi lại với chuỗi chính.

Bạn có thể có cách tiếp cận tương tự. Mỗi lớp thông báo sẽ cần biết cách xử lý chính nó và có thể tự đăng ký với ứng dụng lớn hơn. Điều này sẽ đơn giản hóa rất nhiều mã và loại bỏ các khớp nối chặt chẽ.

+0

Cảm ơn câu trả lời. Chúng tôi đã thử một số thời gian trước đây, nhưng nó chỉ thay đổi sự phức tạp cho các hàm xử lý - thay vì 99 hàm xử lý nhỏ, bạn có 20 hàm xử lý rất lớn. Bất kỳ ý tưởng tốt hơn? –

5

Bạn cũng có thể sử dụng một số loại khung công tác IoC, như Spring.NET, để chèn từ điển. Bằng cách này, nếu bạn nhận được một loại tin nhắn mới, bạn không cần phải biên dịch lại trung tâm này - chỉ cần thay đổi một tập tin cấu hình.


Ví dụ dài chờ đợi:

Tạo một ứng dụng giao diện điều khiển mới, được đặt tên Ví dụ, và thêm điều này:

using System; 
using System.Collections.Generic; 
using Spring.Context.Support; 

namespace Example 
{ 
    internal class Program 
    { 
     private static void Main(string[] args) 
     { 
      MessageBroker broker = (MessageBroker) ContextRegistry.GetContext()["messageBroker"]; 
      broker.Dispatch(null, new Type1EventArgs()); 
      broker.Dispatch(null, new Type2EventArgs()); 
      broker.Dispatch(null, new EventArgs()); 
     } 
    } 

    public class MessageBroker 
    { 
     private Dictionary<Type, object> handlers; 

     public Dictionary<Type, object> Handlers 
     { 
      get { return handlers; } 
      set { handlers = value; } 
     } 

     public void Dispatch<T>(object sender, T e) where T : EventArgs 
     { 
      object entry; 
      if (Handlers.TryGetValue(e.GetType(), out entry)) 
      { 
       MessageHandler<T> handler = entry as MessageHandler<T>; 
       if (handler != null) 
       { 
        handler.HandleMessage(sender, e); 
       } 
       else 
       { 
        //I'd log an error here 
        Console.WriteLine("The handler defined for event type '" + e.GetType().Name + "' doesn't implement the correct interface!"); 
       } 
      } 
      else 
      { 
       //I'd log a warning here 
       Console.WriteLine("No handler defined for event type: " + e.GetType().Name); 
      } 
     } 
    } 

    public interface MessageHandler<T> where T : EventArgs 
    { 
     void HandleMessage(object sender, T message); 
    } 

    public class Type1MessageHandler : MessageHandler<Type1EventArgs> 
    { 
     public void HandleMessage(object sender, Type1EventArgs args) 
     { 
      Console.WriteLine("Type 1, " + args.ToString()); 
     } 
    } 

    public class Type2MessageHandler : MessageHandler<Type2EventArgs> 
    { 
     public void HandleMessage(object sender, Type2EventArgs args) 
     { 
      Console.WriteLine("Type 2, " + args.ToString()); 
     } 
    } 

    public class Type1EventArgs : EventArgs {} 

    public class Type2EventArgs : EventArgs {} 
} 

Và một file app.config:

<?xml version="1.0" encoding="utf-8" ?> 
<configuration> 
    <configSections> 
    <sectionGroup name="spring"> 
     <section name="context" type="Spring.Context.Support.ContextHandler, Spring.Core"/> 
     <section name="objects" type="Spring.Context.Support.DefaultSectionHandler, Spring.Core"/> 
    </sectionGroup> 
    </configSections> 

    <spring> 
    <context> 
     <resource uri="config://spring/objects"/> 
    </context> 
    <objects xmlns="http://www.springframework.net"> 

     <object id="messageBroker" type="Example.MessageBroker, Example"> 
     <property name="handlers"> 
      <dictionary key-type="System.Type" value-type="object"> 
      <entry key="Example.Type1EventArgs, Example" value-ref="type1Handler"/> 
      <entry key="Example.Type2EventArgs, Example" value-ref="type2Handler"/> 
      </dictionary> 
     </property> 
     </object> 
     <object id="type1Handler" type="Example.Type1MessageHandler, Example"/> 
     <object id="type2Handler" type="Example.Type2MessageHandler, Example"/> 
    </objects> 
    </spring> 
</configuration> 

Output:

 
Type 1, Example.Type1EventArgs 
Type 2, Example.Type2EventArgs 
No handler defined for event type: EventArgs 

Như bạn có thể thấy, MessageBroker không biết về bất kỳ trình xử lý nào và trình xử lý không biết về MessageBroker. Tất cả các bản đồ được thực hiện trong ứng dụng.tệp cấu hình, để nếu bạn cần xử lý một loại sự kiện mới, bạn có thể thêm nó vào tệp cấu hình. Điều này đặc biệt tốt nếu các nhóm khác đang định nghĩa các loại sự kiện và trình xử lý - chúng có thể biên dịch nội dung của chúng trong dll, bạn thả nó vào triển khai của mình và chỉ cần thêm ánh xạ.

Từ điển có giá trị của đối tượng kiểu thay vì MessageHandler<> vì trình xử lý thực không thể được truyền tới MessageHandler<EventArgs>, vì vậy tôi phải hack xung quanh một chút. Tôi nghĩ rằng giải pháp vẫn còn sạch sẽ, và nó xử lý các lỗi ánh xạ tốt. Lưu ý rằng bạn cũng sẽ cần phải tham khảo Spring.Core.dll trong dự án này. Bạn có thể tìm thấy các thư viện here và tài liệu here. Các là có liên quan đến điều này. Cũng lưu ý, không có lý do gì bạn cần sử dụng Spring.NET cho điều này - ý tưởng quan trọng ở đây là tiêm phụ thuộc. Bằng cách nào đó, một cái gì đó sẽ cần phải nói cho các nhà môi giới để gửi tin nhắn của loại a đến x, và sử dụng một container IoC cho tiêm phụ thuộc là một cách tốt để có các nhà môi giới không biết về x, và ngược lại.

Một số khác SO câu hỏi liên quan đến IoC và DI:

+0

Cảm ơn câu trả lời. Cool, nghe có vẻ thú vị.Tôi không rõ ràng về một khía cạnh: có thể khuôn khổ IoC vẫn sử dụng chức năng xử lý của tôi? Ý tôi là, từ điển của tôi bao gồm các khóa EventArg và các giá trị hàm xử lý. Ioc làm việc với các giá trị hàm xử lý vẫn còn? –

+0

Các trình xử lý sẽ cần được tách ra thành các lớp riêng biệt đã triển khai thực hiện một giao diện, và sau đó bạn sẽ gọi phương thức "handleMessage" của chúng. Hoặc, bạn có thể vượt qua trong chuỗi, và sử dụng phản ánh để lấy các phương pháp, mặc dù đó là loại mã mùi. Tôi có thể chỉnh sửa với một ví dụ nếu bạn thích. –

+0

Ok, vì vậy thay vì FooBar biết về 99 loại, chúng tôi có 99 loại biết về FooBar. Tôi rất muốn xem một mẫu. –

0

Tôi không thấy phần còn lại của mã của bạn, nhưng tôi sẽ cố gắng tạo một số lượng nhỏ hơn các lớp Event arg. Thay vào đó, hãy tạo một số tương tự nhau về dữ liệu chứa và/hoặc cách bạn xử lý chúng sau này và thêm trường sẽ cho bạn biết loại sự kiện chính xác nào đã xảy ra (có thể bạn nên sử dụng enum).

Lý tưởng nhất là bạn sẽ không chỉ làm cho constructor này nhiều hơn nữa có thể đọc được, mà còn là cách các thông điệp được xử lý (tin nhắn nhóm đó được xử lý theo một cách tương tự trong một xử lý sự kiện duy nhất)

+0

Cảm ơn câu trả lời. Chris Lively đã đề xuất số lượng nhỏ hơn các lớp sự kiện arg. Chúng tôi đã thử điều đó, và nó chỉ thay đổi sự phức tạp: thay vì 99 hàm xử lý nhỏ, bây giờ chúng ta có 20 hàm xử lý lớn. –

+0

Về "các thư nhóm được xử lý theo cách tương tự trong một trình xử lý sự kiện duy nhất", vâng, chúng tôi làm điều đó. Chúng tôi có khoảng 20 sự kiện args được xử lý theo cách tương tự và được chuyển cho cùng một người xử lý. –

0

Rõ ràng bạn đang cần một cơ chế điều phối: tùy thuộc vào sự kiện bạn nhận được, bạn muốn thực thi mã khác.

Bạn dường như đang sử dụng hệ thống loại để xác định các sự kiện, trong khi đó thực sự có nghĩa là hỗ trợ đa hình. Như Chris Lively gợi ý, bạn có thể chỉ là tốt (mà không lạm dụng hệ thống kiểu) sử dụng một liệt kê để xác định các thông điệp.

Hoặc bạn có thể nắm lấy sức mạnh của hệ thống kiểu và tạo đối tượng Đăng ký nơi mọi loại sự kiện được đăng ký (bởi một phiên bản tĩnh, tệp cấu hình hoặc bất kỳ loại nào). Sau đó, bạn có thể sử dụng mẫu Chuỗi trách nhiệm để tìm trình xử lý thích hợp. Hoặc là trình xử lý tự xử lý, hoặc nó có thể là một Nhà máy, tạo một đối tượng xử lý sự kiện.

Phương thức thứ hai có vẻ hơi không được xác định và vượt qua, nhưng trong trường hợp có 99 loại sự kiện (đã có), nó có vẻ phù hợp với tôi.

+0

Tôi không hiểu ý kiến ​​của bạn. Bạn có thể chỉ cho tôi nhiều tài nguyên hơn hoặc đăng một ví dụ không? –