Question

I have a public property (AllCustomers), which is backed by a private property for lazy loading.

I understand that the public property should be IEnumerable ("program to interfaces, not implementations").

However, I can see two ways to build the private property.

First option, with private List-

private List<Customer> _AllCustomers;
public IEnumerable<Customer> AllCustomers
{
    get
    {
        if (_AllCustomers == null)
        {
            _AllCustomers = DAL.GetAllCustomers().ToList(); 
        }
        return _AllCustomers;
    }
}

Second option, with private IEnumerable-

private IEnumerable<Customer> _AllCustomers;
public IEnumerable<Customer> AllCustomers
{
    get
    {
        if (_AllCustomers == null)
        {
            _AllCustomers = DAL.GetAllCustomers(); 
        }
        return _AllCustomers;
    }
}

I think the first option appears more correct, as it will hit the database once and store the results, whereas the second will result in multiple database hits.

My questions are-

  • Am I correct in my analysis?
  • What are the implications of the different approaches?
  • Is there any time the second option would be preferred?
  • Are there better, more idiomatic ways to express the second option?
Was it helpful?

Solution

There are several levels of "laziness" involved here. One is the inherent laziness of IEnumerable implementations, the other is the lazy implementation you add yourself to your property.

Your first implementation will hit the database once, the first time AllCustomers is accessed. It will construct the query in GetAllCustomers and perform it when you call ToList, storing the results locally.

Your second implementation will also hit the database only once (assuming your LINQ implementation is halfway decent). However, this will be later than in the first scenario - even calling your AllCustomer property will only return an IQueryable, which will only be performed when AllCustomers is actually accessed or enumerated. This might be immediately after, or not - it's a lazier implementation. Again, assuming your LINQ provider isn't too stupid, iterating over the entire collection will still only hit the DB once.

Why should you choose the first option anyway? Because (again, depending on the implementation), iterating over AllCustomers twice might hit the DB twice. Resharper, conveniently enough, warns us when we have a possible multiple enumeration of an IEnumerable. Storing it in a local List will ensure we keep a cached local copy. To make sure this is expressed explicitly in the code, consider exposing an IReadOnlyList instead of an IEnumerable.

OTHER TIPS

Both will hit the DB only once as the _AllCustomers backing field is only NULL when the getter is called for the first time.

As a matter of personal preference I'm usually preferring IEnumerable in my interface and method declarations, however backing fields are concrete classes, e.g. List<stuff>. Makes things like modifying your collections amongst others easier.

I understand that the public property should be IEnumerable ("program to interfaces, not implementations").

This understanding is flawed. The principle of programming to interfaces doesn't say which interface you should program to. In your example. I would expose IList<T>, and make it readonly assuming you don't want consumers to modify the list:

private List<Customer> _AllCustomers;
public IList<Customer> AllCustomers
{
    get
    {
        if (_AllCustomers == null)
        {
            _AllCustomers = DAL.GetAllCustomers().ToList().AsReadOnly(); 
        }
        return _AllCustomers;
    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top