List vs IEnumerable in private, lazy-loaded property

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?

We can't possibly answer without knowing what `DAL.GetAllCustomers()` does. It might return a `List` for example, even if it's *declared* to return `IEnumerable`. — Jon Skeet, Jan 14 '14 at 06:59
@MarcinJuraszek - Thanks, private members updated to fields. — Spongeboy, Jan 15 '14 at 00:09
@JonSkeet - So if DAL.GetAllCustomers() returns a List, then there is no difference between the samples? — Spongeboy, Jan 15 '14 at 00:12
@Spongeboy: Not necessarily. The first version creates a copy of the list. `DAL.GetAllCustomers()` might be caching the list it returns, for example - in which case you could easily demonstrate different behaviour by casting the result of `AllCustomers` back to `List` and modifying it. — Jon Skeet, Jan 15 '14 at 06:48
If you have few customers, those alternatives are ok... but more probably you have a large number of data. in this case, you'll need to use `IQueryable` and paging mechanism (`Skip`, `Take`, `OrderBy`) to avoid out of memory exception. — Jaider, Jun 10 '15 at 19:37

score 1 · Answer 1 · answered Jan 14 '14 at 06:58

1

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.

answered Jan 14 '14 at 06:58

cacau

3,606
3
21
42

3

No, the second one *might* hit the database each time the customers are requested. We can't know, because we don't know how `DAL.GetAllCustomers()` is implemented. It might return a query which will execute each time, or it might return a materialized list... – Jon Skeet Jan 14 '14 at 07:00
1

If GetAllCustomers returns an IQueryable then it will hit DB everytime. – Akash Kava Jan 14 '14 at 07:05
@AkashKava Not necessary. You can return `List` as `IQueryable` using `AsQueryable` method. – MarcinJuraszek Jan 14 '14 at 07:07
@MarcinJuraszek Even after AsQueryable the instance of List is still a List not query to database. – Akash Kava Jan 14 '14 at 07:13
@AkashKava Yes, what makes your *If it return `IQueryable` it will hit DB everytime* invalid. There is a chance that DB will be hit only once even when method is declared to return `IQueryable`. – MarcinJuraszek Jan 14 '14 at 07:15
@MarcinJuraszek It is implied that IQueryable is query to database, no idiot uses List and cast it to IQueryable. – Akash Kava Jan 14 '14 at 07:21

score 1 · Accepted Answer · answered Jan 14 '14 at 06:59

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.

Are you sure about your statement `iterating over the entire collection will still only hit the DB once.`? It is absolutely wrong, LINQ provider returns IQueryable which hit DB on every enumeration. — Akash Kava, Jan 14 '14 at 07:09
@AkashKava You're right, and I was unclear. It will hit the DB once for every enumeration, but not for every iteration of that enumeration. I might have misunderstood the OP's concerns in that case, but I think my edit clears that up. — Avner Shahar-Kashtan, Jan 14 '14 at 07:10
Well even for every iteration, the reader is still open. So enumerating over Query keeps DB connection open. It is still better to call ToList and then enumerate on it. — Akash Kava, Jan 14 '14 at 07:16
That depends on the LINQ provider, as @MarcinJuraszek states in the other answer. — Avner Shahar-Kashtan, Jan 14 '14 at 07:37

score 0 · Answer 3 · answered Jan 14 '14 at 07:37

0

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;
    }
}

answered Jan 14 '14 at 07:37

Joe

122,218
32
205
338

But IList is less permissive than IEnumerable, the "program to interfaces" principle to use the most permissive. I understand IList should be used if the data contract implies that random access is required. – Spongeboy Jan 15 '14 at 00:56
1

@Spongeboy: I can see the argument for being permissive with method parameters. But I would only use a return value of IEnumerable if I wanted to hint to the caller that the implementation may use lazy evaluation. Returning IList or ICollection makes it easier for the caller (Count property, easier to iterate repeatedly). – Joe Jan 15 '14 at 06:29

List vs IEnumerable in private, lazy-loaded property

3 Answers3

Linked