0

I am trying to get my model constructors written so that nullable works, with no warnings, and no having to assign default! - in other words, to do it all right.

BTW, if I'm wrong on any of this, please let me know.

I'm mostly there. What's left is collections. It's not seeing that I am setting a required property in the constructor.

public class Campaign
{
    private string _name;

    public int Id { get; private set; }

    public required string Name
    {
        get => _name;
        set => _name = value.Trim();
    }

    public required State State { get; set; }

    public required ICollection<Event> Events { get; set; }

    public required ICollection<User> Followers { get; set; }

    public bool Enabled { get; set; }

    public bool Closed { get; set; }

    public bool Deleted { get; set; }

    public DateTime Created { get; private set; }

    protected Campaign(int id, string name, DateTime created)
    {
        _name = name;
        Id = id;
        Created = created;
    }

    public Campaign()
    {
        _name = string.Empty;
        Events = new List<Event>();
        Followers = new List<User>();
        Enabled = true;
        Created = DateTime.UtcNow;
    }
}

In my xUnit test it requires that I set Events & Followers:

    var campaign = new Campaign
    {
        Name = "Test Campaign",
        State = state,
        // compile error without the following two lines
        Followers = new List<User>(),
        Events = new List<Event>()
    };

Why is it requiring this? Is the trick to get rid of the required in the declaration for those two properties? And if so, is there any downside to that?

If I get rid of the required, then in the protected constructor I need to set those collection properties to default! which I am not wild on (trying to not use that construct). But if the only pace I do that is a constructor used only be EF, I can live with that.

The protected (I read somewhere that private won't work for EF in some cases) constructor is for EF only.

Update: As I try different things, I am leaning toward the following.

First, each model has 2 constructors. A protected one for EF and a public one that has no parameters. Why no parameters? Because the only use cases I see for my (as opposed to EF) creating a model object is I am going to build one up to insert in the DB. In this case my approach is to create object, and then populate it.

In the public/no params constructor, I set all non-nullable strings to Empty and all collections to empty lists. So everything is in the correct NRT state.

I then remove the required keyword from those properties because each is set either in my constructor or by EF. So there's no need to remind/enforce setting it.

If I'm missing something or there's a better way, please let me know.

David Thielen
  • 28,723
  • 34
  • 119
  • 193
  • 1
    Collection navigation properties should always be initialized - they could be empty, but never null. Contrary, reference navigation properties must never be initialized with `new()` (see https://stackoverflow.com/questions/20757594/ef-codefirst-should-i-initialize-navigation-properties). In general, NRT does not work well with reference navigation properties, since even they are required in the database, they might not be loaded. EFC team vision here https://learn.microsoft.com/en-us/ef/core/miscellaneous/nullable-reference-types. – Ivan Stoev Mar 19 '23 at 16:37
  • @IvanStoev That link you referenced said that leaving a collection navigation property null is a valid approach - use it to verify that EF loaded the collection. And it said that initializing it to an empty collection can in some cases cause EF to not populate it??? I've got all this working fine and I think correctly if I set them to default! in the protected EF only constructor. Will this approach bite me in the ass? Or will EF always initialize those collections? – David Thielen Mar 19 '23 at 16:50
  • Ok, the first link could be a bit outdated. But the second is directly to EF Core docs. There you can read *"**Note** Collection navigations, which contain references to multiple related entities, should always be non-nullable. An empty collection means that no related entities exist, but the list itself should never be null."*. I think you are mixing collection with reference navigations, however they are totally different as nullable semantics and loading behavior. – Ivan Stoev Mar 19 '23 at 17:57
  • @IvanStoev I read that as saying that EF, when loading and populating a model, will insure that the collection is non-null. Not that the programmer should initialize it to a list in the constructor. I see how it could be interpreted that it should be initialized in the constructor though. That statement, with where it is in the document, is ambiguous. – David Thielen Mar 19 '23 at 18:25
  • For collections I always initialize them in-line: `public required ICollection Events { get; set; } = new List();` (or `new HashSet()`) which just saves assuming/checking for initialization in a constructor. Singular navigation references on the other hand are a pain as you absolutely should *not* initialize them. For these I declare a protected constructor and slap the disable/restore 8618 around it. It's by no means ideal, but I don't want the warning "noise" and I'm not going to lose sleep over making this toss of the coin feature happy. :) – Steve Py Mar 19 '23 at 21:57

0 Answers0