Why use a public property instead of a function to expose a custom collection<T>

Question

During a code review I looked at set of repository classes (in vb.net). I am used to seeing these repository classes full of functions that return collections (among other things) of your domain objects. However, this repository had 1 public property and 1 private variable that looked something like this:

Private _item as Collection (of Customer)

Public Item as Collection (of Customer)
   Get...
   Set...

In the "Get", there is code that gets a the Customers from the DAL and loads it in the private _item.

What would the benefits be in using a property (customerRepository.Item) instead of plain old function (customerRepository.GetAllCustomers)? The "Property" way looks odd to me but odd doesn't always mean wrong.

Answer 1

If you mean that in Get (instead of the Setter) "there is code that gets a the Customers from the DAL and loads it in the private _item", then I've seen code where that is done, but there is a check to see if the private _item is null. This then turns into a cache and it's only read from the DAL the first time it's accessed. Every other time it is returned from the private _item directly.

Of course, you can also do the cache scenario inside GetAllCustomers.

Answer 2

I agree that putting an operation in the setter that accesses any kind of DAL is bad practice.

According to the MSDN Property Usage Guidelines, methods must be used when:

• The operation is expensive enough that you want to communicate to the user that they should consider caching the result.
• Obtaining a property value using the get accessor would have an observable side effect.

So clearly, using a property for the above is a violation of that guideline.

Answer 3

In this example, the getter is returning the whole collection and the user is allowed to get items from it. In a Repository Pattern the repository IS the collection, and you interact with it with collection semantics to get a particular instance of the entity the repository is supposed to be holding.

The danger in this implementation is that users of this API have the ability of replacing the collection with another collection. This is bad practice, in my opinion