EF codefirst : Should I initialize navigation properties?

Question

I had seen some books(e.g programming entity framework code first Julia Lerman) define their domain classes (POCO) with no initialization of the navigation properties like:

public class User
{
    public int Id { get; set; }
    public string UserName { get; set; }

    public virtual ICollection<Address> Address { get; set; }
    public virtual License License { get; set; }
}

some other books or tools (e.g Entity Framework Power Tools) when generates POCOs initializes the navigation properties of the the class, like:

public class User
{
    public User()
    {
        this.Addresses = new IList<Address>();
        this.License = new License();
    }
    public int Id { get; set; }
    public string UserName { get; set; }

    public virtual ICollection<Address> Addresses { get; set; }
    public virtual License License { get; set; }
}

Q1: Which one is better? why? Pros and Cons?

Edit:

public class License
{
    public License()
    {
        this.User = new User();
    }
    public int Id { get; set; }
    public string Key { get; set; }
    public DateTime Expirtion { get; set; }

    public virtual User User { get; set; }
}

Q2: In second approach there would be stack overflow if the `License` class has a reference to `User` class too. It means we should have one-way reference.(?) How we should decide which one of the navigation properties should be removed?

Bandler

There's no risk of a stack overflow. You are not creating entity instances but only an instance of the list. – Password

No, you are confusing initializing a list with initializing items on the list. As long as you initialize list, its is empty, there are no elements. – Password

@WiktorZychla:Oh dude, you are right about collections, they shouldn't cause stackoverflow because we just new the collection not the items of collection. My question was about a one to one relation like relation between person and account which should cause stackoverflow indeed. I updated my question. – Bandler

Initializing virtual properties in the constructor is really bad bad bad. Frankly, I'm rather surprised to see this presented as a solution by authors who should know better. Because the base portion of the object is constructed first, the subclass constructor is yet to run when the these virtual members are accessed. If the virtual methods are overridden and their implementation depends on initialization in the subclass constructor, they'll break. EF works by creating a subclass at runtime and overriding the virtual members. There's significant risk of this problem occurring. – Stockman

I've always considered using virtual members in EF as convenient but ultimately flawed. It can cause many more hits to the database than the developer anticipated. Better to think about what you want to load and .Include it when you first hit the DB. – Stockman

@Stockman you are right, however, what people should be avoiding is using virtual public class properties, child classes should access only via public getters or setters. Also, concrete classes should not be extended anyway – Fran

A reference is an entity. A collections contains entities. This means that initializing a collection is meaningless in terms of business logic: it does not define an association between entities. Setting a reference does. So it's purely a matter of preference whether or not, or how, you initialize embedded lists. As for the "how", some people prefer lazy initialization: private ICollection<Address> _addresses; public virtual ICollection<Address> Addresses { get { return this._addresses ?? (this._addresses = new HashSet<Address>()); } – Aultman

Answer 1

Collections: It doesn't matter.

There is a distinct difference between collections and references as navigation properties. A reference is an entity. A collections contains entities. This means that initializing a collection is meaningless in terms of business logic: it does not define an association between entities. Setting a reference does.

So it's purely a matter of preference whether or not, or how, you initialize embedded lists.

As for the "how", some people prefer lazy initialization:

private ICollection<Address> _addresses;

public virtual ICollection<Address> Addresses
{ 
    get { return this._addresses ?? (this._addresses = new HashSet<Address>());
}

It prevents null reference exceptions, so it facilitates unit testing and manipulating the collection, but it also prevents unnecessary initialization. The latter may make a difference when a class has relatively many collections. The downside is that it takes relatively much plumbing, esp. when compared to auto properties without initialization. Also, the advent of the null-propagation operator in C# has made it less urgent to initialize collection properties.

...unless explicit loading is applied

The only thing is that initializing collections makes it hard to check whether or not a collection was loaded by Entity Framework. If a collection is initialized, a statement like...

var users = context.Users.ToList();

...will create User objects having empty, not-null Addresses collections (lazy loading aside). Checking whether the collection is loaded requires code like...

var user = users.First();
var isLoaded = context.Entry(user).Collection(c => c.Addresses).IsLoaded;

If the collection is not initialized a simple null check will do. So when selective explicit loading is an important part of your coding practice, i.e. ...

if (/*check collection isn't loaded*/)
    context.Entry(user).Collection(c => c.Addresses).Load();

...it may be more convenient not to initialize collection properties.

Reference properties: Don't

Reference properties are entities, so assigning an empty object to them is meaningful.

Worse, if you initiate them in the constructor, EF won't overwrite them when materializing your object or by lazy loading. They will always have their initial values until you actively replace them. Worse still, you may even end up saving empty entities in the database!

And there's another effect: relationship fixup won't occcur. Relationship fixup is the process by which EF connects all entities in the context by their navigation properties. When a User and a Licence are loaded separately, still User.License will be populated and vice versa. Unless of course, if License was initialized in the constructor. This is also true for 1:n associations. If Address would initialize a User in its constructor, User.Addresses would not be populated!

Entity Framework core

Relationship fixup in Entity Framework core (2.1 at the time of writing) isn't affected by initialized reference navigation properties in constructors. That is, when users and addresses are pulled from the database separately, the navigation properties are populated.
However, lazy loading does not overwrite initialized reference navigation properties.

Additionally, initializing a reference navigation property...

• May prevent Include from working properly.

• Will cause errors in seeding by using HasData. It wil throw an exception like

  The seed entity for entity type 'User' cannot be added because it has the navigation 'License' set. To seed relationships, add the entity seed to 'User' and specify the foreign key values {'LicenseId'}.`

So, in conclusion, also in EF-core, initializing reference navigation properties in constructors may cause trouble. Don't do it. It doesn't make sense anyway.

Presentiment

wow, never seen like this, seems good idea but never seen in any project/book. +1 for special answer. But I'm scared of using this. I update my question, would you do the same technique for non-collecton reference properties? – Bandler

No, because it's no use to set reference properties with default objects. These references have business meaning, so they should either be null or refer to meaningful objects that are set on purpose. – Presentiment

Off topic and late in the day, but, why would you use a HashSet in particular here? – Beowulf

@Beowulf It's one of several options. HashSet is optimized for quick lookup and it is guaranteed to contain unique objects. EF generates HashSets by default with t4 or code-first from database. – Presentiment

any explanation as to why set is not included? – Kianakiang

I am using EF Core. When I don't initialize the collection navigation property Addresses and invoke context.Users.Include(u=>u.Addresses), I find that the Addressess is not-null but an empty list for users with no addresses. Is it always correct? – Cockiness

@ArtificialStupidity That's expected behavior. – Presentiment

Answer 2

In all my projects I follow the rule - "Collections should not be null. They are either empty or have values."

First example is possible to have when creation of these entities is responsibility of third-part code (e.g. ORM) and you are working on a short-time project.

Second example is better, since

• you are sure that entity has all properties set
• you avoid silly NullReferenceException
• you make consumers of your code happier

People, who practice Domain-Driven Design, expose collections as read-only and avoid setters on them. (see What is the best practice for readonly lists in NHibernate)

Q1: Which one is better? why? Pros and Cons?

It is better to expose not-null colections since you avoid additional checks in your code (e.g. Addresses). It is a good contract to have in your codebase. But it os OK for me to expose nullable reference to single entity (e.g. License)

Q2: In second approach there would be stack overflow if the License class has a reference to User class too. It means we should have one-way reference.(?) How we should decide which one of the navigation properties should be removed?

When I developed data mapper pattern by myself I tryed to avoid bidirectional references and had reference from child to parent very rarely.

When I use ORMs it is easy to have bidirectional references.

When it is needed to build test-entity for my unit-tests with bidirectional reference set I follow the following steps:

1. I build parent entity with emty children collection.
2. Then I add evey child with reference to parent entity into children collection.

Insted of having parameterless constructor in License type I would make user property required.

public class License
{
    public License(User user)
    {
        this.User = user;
    }

    public int Id { get; set; }
    public string Key { get; set; }
    public DateTime Expirtion { get; set; }

    public virtual User User { get; set; }
}

Spinthariscope

I have updated my question, would you please check second question, and add more details to your answer. – Bandler

I've added answer on you second question. The main idea that collections shouldn't be nullable, but it is possible to avoid reference from child to the parent. – Spinthariscope

Thanks for your answer. Would you please explain what you mean by First example is possible to have when creation of these entities is responsibility of third-part code and test-entity? And would you please add your Email address to your profile.Thanks again. – Bandler

test-entity is a dummy or stub entity that you use in your unit tests. In this case third-part code is ORM. So, First example is possible when you use ORM that initialises your entities/collections, when you have generic infrastructure that makes sure that collections are not null. – Spinthariscope

Answer 3

It's redundant to new the list, since your POCO is depending on Lazy Loading.

Lazy loading is the process whereby an entity or collection of entities is automatically loaded from the database the first time that a property referring to the entity/entities is accessed. When using POCO entity types, lazy loading is achieved by creating instances of derived proxy types and then overriding virtual properties to add the loading hook.

If you would remove the virtual modifier, then you would turn off lazy loading, and in that case your code no longer would work (because nothing would initialize the list).

Note that Lazy Loading is a feature supported by entity framework, if you create the class outside the context of a DbContext, then the depending code would obviously suffer from a NullReferenceException

HTH

Morphogenesis

I quickly learnt to disable lazy loading in many scenario's, because it forces you to get your data efficiently. So yes: I always initialize the Navigation Collections (and the Complex Types), but never the singular Reference properties. – Feature

If true, this seems to be the best answer, i.e. using lazy loading (the case of the question) means the initialization is done by EF, not using lazy loading requires initialization in user's code. This answer is clear and up to the point, other answers try to circumvent the actual question and talk about "what is usually done" because they don't know what EF actually does. A reference to the documentation would be good though, because there are numerous tutorials using lazy loading with manual initialization and a question about a null reference. – Xerox

Answer 4

The other answers fully answer the question, but I'd like to add something since this question is still relevant and comes up in google searches.

When you use the "code first model from database" wizard in Visual Studio all collections are initialized like so:

public partial class SomeEntity
{
    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2214:DoNotCallOverridableMethodsInConstructors")]
    public SomeEntity()
    {
        OtherEntities = new HashSet<OtherEntity>();
    }

    public int Id { get; set; }

    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")]
    public virtual ICollection<OtherEntity> OtherEntities { get; set; }
}

I tend to take wizard output as basically being an official recommendation from Microsoft, hence why I'm adding to this five-year-old question. Therefore, I'd initialize all collections as HashSets.

And personally, I think it'd be pretty slick to tweak the above to take advantage of C# 6.0's auto-property initializers:

    public virtual ICollection<OtherEntity> OtherEntities { get; set; } = new HashSet<OtherEntity>();

Smirk

Answer 5

Q1: Which one is better? why? Pros and Cons?

The second variant when virtual properties are set inside an entity constructor has a definite problem which is called "Virtual member call in a constructor".

As for the first variant with no initialization of navigation properties, there are 2 situations depending on who / what creates an object:

1. Entity framework creates an object
2. Code consumer creates an object

The first variant is perfectly valid when Entity Framework creates a object, but can fail when a code consumer creates an object.

The solution to ensure a code consumer always creates a valid object is to use a static factory method:

1. Make default constructor protected. Entity Framework is fine to work with protected constructors.

2. Add a static factory method that creates an empty object, e.g. a User object, sets all properties, e.g. Addresses and License, after creation and returns a fully constructed User object

This way Entity Framework uses a protected default constructor to create a valid object from data obtained from some data source and code consumer uses a static factory method to create a valid object.

Frit

Answer 6

I use the answer from this Why is my Entity Framework Code First proxy collection null and why can't I set it?

Had problems with constructor initilization. Only reason I do this is to make test code easier. Making sure collection is never null saves me constantly initialising in tests etc

Culhert