199

Suppose you have a class Person :

public class Person
{
   public string Name { get; set;}
   public IEnumerable<Role> Roles {get; set;}
}

I should obviously instantiate the Roles in the constructor. Now, I used to do it with a List like this :

public Person()
{
   Roles = new List<Role>();
}

But I discovered this static method in the System.Linq namespace

IEnumerable<T> Enumerable.Empty<T>();

From MSDN:

The Empty(TResult)() method caches an empty sequence of type TResult. When the object it returns is enumerated, it yields no elements.

In some cases, this method is useful for passing an empty sequence to a user-defined method that takes an IEnumerable(T). It can also be used to generate a neutral element for methods such as Union. See the Example section for an example of this use of

So is it better to write the constructor like that? Do you use it? Why? or if not, Why not?

public Person()
{
   Roles = Enumerable.Empty<Role>();
}
Wai Ha Lee
  • 8,598
  • 83
  • 57
  • 92
Stéphane
  • 11,755
  • 7
  • 49
  • 63
  • 1
    This is a Data class. I intend to use this class as a Model class when implementing a repository pattern with Entity Framework 4.0 (playing around...). so I think it's fine to have a public setter here, isn't it? – Stéphane Dec 12 '09 at 17:46
  • 1
    serbech, how will you Add a role (inside Person) when other code can installed any kind of IEnumerable derived class for the list? What will you cast it to? – H H Dec 12 '09 at 18:03
  • 1
    I see your point, I should probably have a IList internally, or even a List here? And that would just remove my original problem... – Stéphane Dec 12 '09 at 18:43
  • I use Enumerable.Empty in unit testing to indicate unhappy path tests to help communicate the intent. As many have already pointed it out the bonus is no allocation on the GC which helps when your unit tests number in the hundreds or more. – jjhayter Jun 09 '15 at 16:51
  • I agree that you'd want to remove the `set` and just use the `IList` interface directly. If however you want to keep it as `IEnumerable` with the `set`, *really* it seems you should init it to `null`. Just the same way you don't need to init `Name` to an empty string--since it's got a `set`, you just init to `null` and have the client code `set` it. – Dax Fohl Nov 07 '16 at 16:58
  • 3
    Now one can also use `Array.Empty()` which is explicitly an array and hence an `IList`. In Core it's in fact the same array, with only the type exposed differing. – Jon Hanna Jan 28 '17 at 16:47
  • This is probably long overdue, but if you don't need the indexer `[]` you might be better off using `ICollection` instead of `IList`. See also https://stackoverflow.com/questions/9855693/returning-ilist-vs-icollection-vs-collection#9855734 – Didii Jul 31 '18 at 07:08

6 Answers6

255

I think most postings missed the main point. Even if you use an empty array or empty list, those are objects and they are stored in memory. The Garbage Collector has to take care of them. If you are dealing with a high throughput application, it could be a noticeable impact.

Enumerable.Empty does not create an object per call thus putting less load on the GC.

If the code is in low-throughput location, then it boils down to aesthetic considerations though.

Kris van der Mast
  • 16,343
  • 8
  • 39
  • 61
Vadym Chekan
  • 4,977
  • 2
  • 31
  • 24
117

I think Enumerable.Empty<T> is better because it is more explicit: your code clearly indicates your intentions. It might also be a bit more efficient, but that's only a secondary advantage.

Roman Marusyk
  • 23,328
  • 24
  • 73
  • 116
Tommy Carlier
  • 7,951
  • 3
  • 26
  • 43
  • 7
    Yes, making it clear that you mean this to be empty vs. something that might be added to later is good for your code. – Jay Bazuzi Dec 12 '09 at 17:43
  • 32
    Yes. Enumerable<>.Empty does avoid allocating a new object, which is a small bonus. – Neil Jan 04 '10 at 18:38
  • 13
    sidenote @NeilWhitaker: it's Enumerable.Empty not Enumerable.Empty (that one drove me crazy once...) – santa Apr 11 '14 at 11:49
  • 1
    Agreed. To add; if you need to initialize an `IEnumerable`...why initialize it with more firepower than you need. `List` is a more complex object which implements `IEnumerable`, among many other things. I say initialize properties in the most basic way possible and use `Enumerable.Empty()` in this case as well. – Craig Sep 18 '15 at 12:22
36

On the performance front, let's see how Enumerable.Empty<T> is implemented.

It returns EmptyEnumerable<T>.Instance, which is defined as:

internal class EmptyEnumerable<T>
{
    public static readonly T[] Instance = new T[0];
}

Static fields on generic types are allocated per generic type parameter. This means that the runtime can lazily create these empty arrays only for the types user code needs, and reuse the instances as many times as needed without adding any pressure on the garbage collector.

To wit:

Debug.Assert(ReferenceEquals(Enumerable.Empty<int>(), Enumerable.Empty<int>()));
Drew Noakes
  • 300,895
  • 165
  • 679
  • 742
  • 4
    All this while I thought the implementation was like `public static IEnumerable Empty() { yield break; }` MS knows it best, probably a cached `new T[0]` is more efficient. – nawfal Sep 15 '18 at 00:34
  • Verified, the implementation is the same in .NET Core 2.2 (just the class names are different). – nawfal Sep 15 '18 at 01:28
13

Assuming you actually want to populate the Roles property somehow, then encapsulate that by making it's setter private and initialising it to a new list in the constructor:

public class Person
{
    public string Name { get; set; }
    public IList<Role> Roles { get; private set; }

    public Person()
    {
        Roles = new List<Role>();
    }
}

If you really really want to have the public setter, leave Roles with a value of null and avoid the object allocation.

Neil Barnwell
  • 41,080
  • 29
  • 148
  • 220
  • 1
    I would recommend `ICollection` over `IList` – CervEd Mar 12 '21 at 07:30
  • You should never let collections be null. It provides no benefit and only leads to senselessly needing additional null checks all over your code, or worse, exceptions due to forgetting these. Initializing it with `Enumerable.Empty()` or `Array.Empty()` likewise also avoids object allocations. – JvS Oct 14 '22 at 08:39
7

The problem with your approach is that you can't add any items to the collection - I would have a private structure like list and then expose the items as an Enumerable:

public class Person
{
    private IList<Role> _roles;

    public Person()
    {
        this._roles = new List<Role>();
    }

    public string Name { get; set; }

    public void AddRole(Role role)
    {
        //implementation
    }

    public IEnumerable<Role> Roles
    {
        get { return this._roles.AsEnumerable(); }
    }
}

If you intend some other class to create the list of roles (which I wouldn't recommend) then I wouldn't initialise the enumerable at all in Person.

Lee
  • 142,018
  • 20
  • 234
  • 287
  • that's a good point. I would have find that out quickly in my implementation in fact :) – Stéphane Dec 12 '09 at 17:38
  • I would make _roles readonly, and I don't see the need for the call to AsEnumerable(). Otherwise, +1 – Kent Boogaart Dec 12 '09 at 17:56
  • 1
    @Kent - I agree with readonly. As for AsEnumerable, it's not really needed but it prevents Roles being cast to IList and modified. – Lee Dec 12 '09 at 18:07
7

The typical problem with exposing the private List as an IEnumerable is that the client of your class can mess with it by casting. This code would work:

  var p = new Person();
  List<Role> roles = p.Roles as List<Role>;
  roles.Add(Role.Admin);

You can avoid this by implementing an iterator:

public IEnumerable<Role> Roles {
  get {
    foreach (var role in mRoles)
      yield return role;
  }
}
Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • 7
    If a developer casts your IEnumerable to a List, then that's his problem and not yours. If you change the internal implementation in the future (so it's no longer List), it's not your fault that the developer's code breaks. You can't stop people from writing crappy code and abusing your API, so I don't think it's worth spending a lot of effort on. – Tommy Carlier Dec 12 '09 at 17:52
  • 4
    This is not about breaking code, it is about exploiting code. – Hans Passant Dec 12 '09 at 18:03
  • 2
    I agree with @Hans if it's a security concern, and with @Tommy in all other cases. Naughty developers always make that one fatal mistake. – TrueWill Apr 15 '11 at 15:24
  • 1
    And the enumeration part can be writter using the AsEnumerable() extension method. (similar to Lee's answer) – Stéphane May 12 '11 at 09:05
  • 1
    @Stephane no, because `AsEnumerable()` will just give you the `List` again, so if you're hoping to guarantee it can't be cast back, you've gained nothing. – Jon Hanna Sep 04 '15 at 14:33