2010-07-01 38 views
6

Tôi đang tái cấu trúc một chút mã truy cập dữ liệu C# từ nhà phát triển trước và tò mò về mẫu mà anh ấy đã sử dụng.Bất kỳ lợi thế nào đối với đối tượng.GetObject (i) trên đối tượng [i]?

Mã ban đầu được trưng bày các bộ sưu tập (mảng) của một loạt các đối tượng kinh doanh kiểu ActiveRecord - về cơ bản các đối tượng bao quanh các trường cơ sở dữ liệu. Tôi đang thay đổi mảng thành danh sách chung, nhưng khía cạnh của mã tôi tò mò là nhà phát triển trước đó đã Nhận các phương thức cho từng loại đối tượng mà anh ta gói, như vậy:

public Thing GetThing(int i) { 
    return things[i]; 
} 

Có một số của những phương pháp này, và tôi không thể cho cuộc sống của tôi nghĩ về bất kỳ lợi thế có thể có của việc sử dụng cơ chế đó đơn giản chỉ đề cập đến những thứ [i] trực tiếp. Giả sử, vì lợi ích của đối số, rằng mọi thứ là tài sản công cộng, không phải là trường công khai (trong trường hợp này nó thực sự là thuộc tính được tự động triển khai, vì vậy giả định đó thực sự là đúng).

Tôi có thiếu thứ gì đó hiển nhiên không? Hoặc thậm chí một cái gì đó bí truyền?

CẬP NHẬT tôi có lẽ nên làm rõ rằng những bộ sưu tập hiện đang truy cập từ bên trong cho vòng:

for (int i = 0; i < thingsCount; i==) { 
    dosomthing(GetThing(i)); 
    dosomethingelse(GetThing(i)); 
} 

mà tôi đang refactoring để:

for (int i = 0; i < thingsCount; i==) { 
    Thing thing = things[i]; 
    dosomthing(thing); 
    dosomethingelse(thing); 
} 

và thậm chí sử dụng những thứ. cho mỗi().

Trả lời

6

Tôi không biết nếu nó là hiển nhiên, nhưng tôi nghĩ rằng bạn đang thiếu một cái gì đó.

Giả sử thingsIList<Thing>. Sau đó hiển thị trực tiếp (như Things) sẽ cho phép mã gọi để gọi Add, Insert, RemoveAt, v.v. Có thể nhà phát triển trước không muốn cho phép điều này (và tôi chắc chắn có nhiều lý do chính đáng cho điều đó).

Thậm chí giả sử đó là Thing[] (vì vậy Add, v.v.) sẽ không khả dụng), việc hiển thị nó sẽ vẫn cho phép mã gọi để thực hiện điều gì đó như obj.Things[0] = new Thing();. thực hiện.

Bạn có thể phơi bày Things làm ReadOnlyCollection<Thing> sẽ xử lý hầu hết các sự cố này. Nhưng những gì nó đi xuống là: nếu nhà phát triển chỉ muốn cho phép mã gọi để truy cập các mục theo chỉ mục - không có gì khác - sau đó cung cấp một phương thức GetThing duy nhất làm phương tiện, trung thực, ý nghĩa nhất.

Bây giờ, được cấp, cũng có tùy chọn này: triển khai thuộc tính this[int] chỉ với một người truy cập get. Nhưng điều đó chỉ có ý nghĩa nếu lớp học chủ yếu là tập hợp các đối tượng độc quyền (ví dụ: không có cũng một bộ sưu tập của một số khác loại đối tượng bạn muốn cung cấp quyền truy cập trong lớp).

Tất cả đã nói, tôi nghĩ cách tiếp cận GetThing là khá tốt.

Điều đó nói rằng, từ cách bạn diễn đạt câu hỏi của bạn, họ sẽ thốt lên các nhà phát triển trước đó thực hiện một số quyết định khá nghèo khác:

  1. Nếu anh/cô ấy cũng tiếp xúc với bộ sưu tập things trực tiếp như một tài sản công cộng, tốt, sau đó ... mà đánh bại toàn bộ mục đích của phương pháp GetThing, phải không? Kết quả chỉ đơn giản là một giao diện cồng kềnh (tôi thường nghĩ đó không phải là một dấu hiệu tuyệt vời khi bạn có nhiều phương pháp để thực hiện chính xác điều tương tự, trừ khi chúng được ghi rõ là bí danh cho một số lý do chính đáng). [Cập nhật: Có vẻ như nhà phát triển trước đã làm không làm điều này. Tốt.]
  2. Có vẻ như nhà phát triển trước cũng đã nội bộ truy cập các mục từ things bằng phương pháp GetThing, điều này thật ngớ ngẩn (theo ý kiến ​​của tôi). Tại sao giới thiệu chi phí vô nghĩa của các cuộc gọi phương thức bổ sung bằng cách sử dụng giao diện công khai của lớp từ bên trong lớp đó? Nếu bạn đang ở trong lớp, bạn đã là bên trong triển khai và có thể truy cập dữ liệu riêng tư/được bảo vệ mà bạn muốn - không cần giả vờ cách khác.
+0

Suy nghĩ qua phản ứng của Elpezmuerto tôi đã đi đến kết luận chính xác này - mặc dù tôi đang phơi bày * bộ sưu tập * như các thuộc tính với {get; private set;} một khi tôi nhận được bộ sưu tập, tôi có thể sửa đổi nó bằng cách gọi mã theo những cách không có ý nghĩa gì cả. Nhà phát triển trước có những điểm tiếp xúc được bảo vệ, vì vậy điểm 1 của bạn không liên quan đến trường hợp của tôi, nhưng anh ấy gọi cho GetThing (i) nhiều lần trong mỗi vòng lặp mà tôi đồng ý không có ý nghĩa. Có vẻ như refactor tốt nhất là để lại phương thức GetThing(), nhưng chỉ gọi chúng một lần. Cảm ơn! – cori

+0

@cori: Có, một lỗi thực sự phổ biến mà nhà phát triển thực hiện trong việc thiết kế lớp học của họ là hiển thị trực tiếp các bộ sưu tập có thể thay đổi mà không tính đến điều gì có thể xảy ra nếu/khi các bộ sưu tập đó được sửa đổi. Nếu bạn chỉ muốn bật mã gọi để liệt kê và không sửa đổi bộ sưu tập, hãy hiển thị một 'IEnumerable 'và không, ví dụ:' Danh sách '! –

+0

@cori: Tôi sẽ thừa nhận nó khá bực bội, tuy nhiên, khi bạn muốn cho phép truy cập ngẫu nhiên theo chỉ mục nhưng không phải tất cả các tính năng khác của giao diện 'IList ' hoặc thậm chí là 'T []' (cho phép bạn đặt các mục) , không chỉ nhận được chúng). Điều này thực sự là một cái gì đó tôi hỏi về [một câu hỏi cũ của riêng tôi] (http://stackoverflow.com/questions/2110440/why-is-there-no-iarrayt-interface-in-net) một trong khi trở lại. –

2

Có hai điều cần lưu ý ở đây. Đầu tiên là bạn muốn giữ các biến đối tượng riêng tư và sử dụng getters và setters để truy cập chúng. Điều này ngăn cản người dùng vô tình thay đổi hoặc sửa đổi các biến đối tượng.

Thứ hai, nó được coi là một quy ước đặt tên tốt nơi các thuật ngữ nhận/tập phải được sử dụng khi một thuộc tính được truy cập trực tiếp. Điều này giúp cải thiện khả năng đọc.

Mặc dù đây là tài sản công cộng trong trường hợp này, việc sử dụng getters và setters cải thiện khả năng đọc và giúp ngăn chặn hành vi không mong muốn. Việc bạn truy cập các biến đối tượng trong vòng lặp không quan trọng, bạn nên tiếp tục sử dụng quy ước GetThing.

Cuối cùng nếu bạn đang truy cập các biến từ bên trong đối tượng, bạn không cần phải sử dụng trình khởi động hoặc thiết lập.

LƯU Ý: thường được coi phong cách tốt của nó để giữ cho các biến đối tượng tư nhân và sử dụng getters/setters cho tất cả các ngôn ngữ

C++ style guidelines

C# style guidelines

Bạn cũng có thể quan tâm đến câu hỏi "getter and setter for class in class c#"

+0

Tôi có thể hiểu rằng logic (mặc dù tôi không thấy bất cứ điều gì tham chiếu nó trong ví dụ C# bạn trỏ đến). Cảm ơn! – cori

+0

@Cori Cập nhật liên kết – Elpezmuerto

2

Bạn có thể muốn phơi bày đối tượng dưới dạng IList<Thing>. Điều này sẽ cung cấp cho bạn khả năng lập chỉ mục mà bạn đang tìm kiếm, nhưng bạn cũng có thể sử dụng nhiều chức năng LINQ, chẳng hạn như tạo danh sách các mục mới dựa trên điều kiện. Thêm IList<T> thực hiện IEnumerable<T> để bạn có thể sử dụng foreach để lặp qua các đối tượng.

ví dụ:trong lớp học của bạn:

public IList<Thing> Things { get; private set; } 

ví dụ: sử dụng:

Thing x = business.Things[3]; 

hoặc

var x = business.Things.Where(t => t.Name == "cori"); 
+0

Đó là chính xác nơi tôi đã được đứng đầu với refactoring vào Danh sách (và có, IList có lẽ sẽ là một lựa chọn tốt hơn). – cori

1

Nếu tôi phải đoán, tôi muốn nói rằng nhà phát triển này đã được sử dụng cho một số ngôn ngữ khác như Java, và không hoàn toàn nhận thức được thực hành tiêu chuẩn trong C#. Danh mục "get [Property]" được sử dụng rất nhiều trong Java, javascript, vv C# thay thế nó bằng các thuộc tính và các bộ chỉ mục. Các thuộc tính đều mạnh mẽ như getters và setters nhưng dễ viết và sử dụng hơn. Lần duy nhất mà bạn thường thấy "Get [gì đó]" trong C# là nếu:

  • các hoạt động có khả năng là đủ đắt tiền mà bạn thực sự muốn lái xe về nhà một thực tế rằng đây là không thể tiếp cận thành viên đơn giản (ví dụ GetPrimeNumbers()) hoặc
  • Bộ sưu tập của bạn thực sự bao gồm nhiều bộ sưu tập được lập chỉ mục. (Ví dụ GetRow(int i) và . Ngay cả trong trường hợp này, đó là phổ biến hơn chỉ đơn giản là tiếp xúc với mỗi người trong các bộ sưu tập lập chỉ mục như một tài sản cho đến bản thân, mà là của một loại được lập chỉ mục ("table.Rows[2]").

Nếu bạn là chỉ truy cập vào các giá trị này trong các vòng for, bộ sưu tập sẽ triển khai IEnumerable<Thing>, giúp bạn truy cập vào các phương pháp LINQ và cấu trúc foreach.Nếu bạn vẫn cần phải có các chỉ số dựa trên chỉ mục, bạn nên cân nhắc sử dụng giao diện của riêng mình mở rộng IEnumerable<T>, nhưng cung cấp thêm:

T this[int i] { get; } 

Bằng cách này, bạn không cung cấp cho người tiêu dùng ấn tượng rằng họ có thể AddRemove đối tượng trong bộ sưu tập này.

Cập nhật

Tôi biết điều này chủ yếu là một vấn đề của phong cách, đó là chủ đề tranh cãi, nhưng tôi thực sự nghĩ rằng giải pháp GetThings không phải là cách đúng để làm việc. Chiến lược sau, trong khi phải mất một công việc ít hơn, xa hơn trong việc giữ với cách mà các lớp .NET tiêu chuẩn và khung được thiết kế:

public class ThingHolderDataAccess 
{ 
    public ThingHolder GetThingHolderForSomeArgs(int arg1, int arg2) 
    { 
     var oneThings = GetOneThings(arg1); 
     var otherThings = GetOtherThings(arg2); 
     return new ThingHolder(oneThings, otherThings); 
    } 
    private IEnumerable<OneThing> GetOneThings(int arg) 
    { 
     //... 
     return new List<OneThing>(); 
    } 
    private IEnumerable<AnotherThing> GetOtherThings(int arg2) 
    { 
     //... 
     return new List<AnotherThing>(); 
    } 
} 

public class ThingHolder 
{ 
    public IIndexedReadonlyCollection<OneThing> OneThings 
    { 
     get; 
     private set; 
    } 

    public IIndexedReadonlyCollection<AnotherThing> OtherThings 
    { 
     get; 
     private set; 
    } 

    public ThingHolder(IEnumerable<OneThing> oneThings, 
         IEnumerable<AnotherThing> otherThings) 
    { 
     OneThings = oneThings.ToIndexedReadOnlyCollection(); 
     OtherThings = otherThings.ToIndexedReadOnlyCollection(); 
    } 
} 

#region These classes can be written once, and used everywhere 
public class IndexedCollection<T> 
    : List<T>, IIndexedReadonlyCollection<T> 
{ 
    public IndexedCollection(IEnumerable<T> items) 
     : base(items) 
    { 
    } 
} 

public static class EnumerableExtensions 
{ 
    public static IIndexedReadonlyCollection<T> ToIndexedReadOnlyCollection<T>(
     this IEnumerable<T> items) 
    { 
     return new IndexedCollection<T>(items); 
    } 
} 

public interface IIndexedReadonlyCollection<out T> : IEnumerable<T> 
{ 
    T this[int i] { get; } 
} 
#endregion 

Sử dụng mã trên có thể trông như thế này:

var things = _thingHolderDataAccess.GetThingHolderForSomeArgs(a, b); 
foreach (var oneThing in things.OneThings) 
{ 
    // do something 
} 
foreach (var anotherThing in things.OtherThings) 
{ 
    // do something else 
} 

var specialThing = things.OneThings[c]; 
// do something to special thing 
+0

cảm ơn bạn đã trả lời tài liệu. Vì mã của tôi chỉ giao dịch với các bộ sưu tập đồng nhất (và nó hoàn toàn nằm dưới sự kiểm soát của tôi và sẽ chỉ được tiêu thụ bởi mã dưới sự kiểm soát của tôi), đây là một chút để bỏ qua chi phí cho các nhu cầu khá đơn giản của tôi .. – cori

+0

@cori: Tôi hoàn toàn hiểu. Khi mã của bạn chỉ được tiêu thụ cục bộ, nó không đáng để thêm vào. – StriplingWarrior

+0

... và trong trường hợp đó, tôi khuyên bạn nên vạch trần những người theo đạo luật như Warren nói. Tôi thấy không có điểm để có một phương pháp GetThing. – StriplingWarrior

0

Giống như các câu trả lời khác đã chỉ ra, đó là vấn đề hạn chế quyền truy cập vào mảng (hoặc danh sách).

Nói chung, bạn không muốn khách hàng của lớp có thể trực tiếp sửa đổi mảng đó. Ẩn việc triển khai danh sách cơ bản cũng cho phép bạn thay đổi triển khai trong tương lai mà không ảnh hưởng đến bất kỳ mã ứng dụng khách nào sử dụng lớp đó.

Các vấn đề liên quan