0

This is razor code but I think the same can happen in most any C# code in an event driven architecture.

private List<User> Users { get; set; }
protected override async Task OnInitializedAsync()
{
    Users = await Context.Users.Include(u => u.Address).ToListAsync();
}

So the above code will initialize Users before it is ever accessed. However, it puts up a warning that a non-nullable variable is not being initialized.

Is this a case of assigning "default!" to it which is my way of saying don't worry it'll be initialized before it is accessed?

Update: This occurs inside a .razor page in the @code part. So it exists while the html is being rendered to pass back to the user's browser. I'm writing this code in an ASP.NET Core Blazor app.

The problem here is the Users object needs to be accessible to all the code in the .razor file. But the code is loaded in the async method. And this method is called as part of the .razor file creation, so it needs to be in there.

David Thielen
  • 28,723
  • 34
  • 119
  • 193
  • _"event driven architecture"_ - uhhhh... I think you're misusing that term. You know how _class invariants_ work, right? – Dai Feb 09 '23 at 23:28
  • 1
    _"Is this a case of assigning "default!" to it which is my way of saying don't worry it'll be initialized before it is accessed?"_ - doing that is just bad class design. Initialization should happen inside the constructor because the constructor exists to enforce class invariants (I know ctors can't be `async`: so if you have async logic or any IO then that should be done by a factory function which _then_ calls the ctor after it has the data that the ctor needs).... – Dai Feb 09 '23 at 23:30
  • ...wich makes me wonder what your `private List Users` property is for if it's ephemeral - is this for a WinForms or other UI component? If so, then `Users` should be an `ObservableCollection { get; } = new();` and _not_ `List { get; set; }` (being `get`-only ensures its reference will never be replaced; to populate it you would clear-and-add the `ObservableCollection` with `UserViewModel` (which would wrap `User` with a WPF/etc-friendly interface)... – Dai Feb 09 '23 at 23:31
  • Ah, you're using Blazor? – Dai Feb 10 '23 at 00:13
  • @Dai yes. And I'm new to it - my previous experience, about 5 years ago, was ASP.NET MVC. So new to Blazor and new to non-nullable in C# (I was a CEO the last 7 years and so no programming all that time). – David Thielen Feb 10 '23 at 00:16
  • In Blazor, the same concepts as WPF apply: your `Users` property should be a `get`-only `ObservableCollection` which is cleared-and-repopulated by your `OnInitializedAsync` method - though presumably you'd want to reload the list at a point in future? – Dai Feb 10 '23 at 00:21
  • @Dai you've given me some very good guidance on how to approach this. thank you. Also, if you want to put all that as the answer, I'm happy to accept it to give you points. – David Thielen Feb 10 '23 at 17:12

1 Answers1

0

To elaborate on my comments, with too much pontification:

  • In OOP, regardless of language (be it Java, C#, C++, Swift, etc), the purpose of a class's constructor is to initialize the object-instance's state from any parameters, and also to establish class invariants.

    • (Btw, don't equate or confuse invariant with immutability: immutability is just a particular kind of invariant, it's perfectly acceptable for a class' objects to be mutable)
  • The principle of class-invariants is more-often-than-lot (in my opinion) lost on so-many (perhaps even most?) users of C#, specifically, because of how the C# language (and its associated documentation and libraries) has evolved towards preferring default (parameterless) constructors, post-construction initialization, and mutable properties - all of which cannot be reconciled with the CS-theoretical, not practical, limits of static analysis which the C# compiler uses for #nullable warnings.

    • The simple fact is only a class's constructor can make guarantees about the state of any of its instance fields (including auto-properties) - whereas (excepting required members in C# 11), a C# object-initializer is evaluated post-construction, is entirely optional, sets entirely arbitrary and non-exhaustive members.
  • Therefore, to make use of #nullable to the full-extent, one must simply get used to getting into the habit of writing parameterized constructors, despite their "expressive redundancy" in most cases (e.g. in a POCO DTO) having to repeat the class's members as ctor parameters and then assign each property in the ctor.

    • Though at least record types in C# 9 simplify this - but record types aren't as immutable as they seem: properties can still be overwritten with invalid values after the ctor has run in an object-initializer, which breaks the concept of class-invariants being enforced by the constructor - I'm really not happy with how that turned out in C# 9, grrr.
  • I appreciate that this isn't possible in many cases, such as with Entity Framework and EF Core, which (as of early 2023) still doesn't support binding database query results to constructor parameters, only to property-setters - but people often are unaware that many other libraries/frameworks do support ctor binding, such as Newtonsoft.Json supports deserializing JSON objects to immutable C# objects via [JsonConstructor] and attaching [JsonProperty] to each ctor parameter, for example.

  • In other cases, namely UI/UX code, where your visual component/control/widget must inherit from some framework-provided base-class (e.g. WinForms' System.Windows.Forms.Control, or WPF's Visual or Control, or in Blazor: Microsoft.AspNetCore.Components.ComponentBase) - you'll find yourself with seemingly contradictory precepts: you can only "initialize" the Control's state /data in the OnLoad/OnInitializedAsync method (not the constructor), but the C# compiler's #nullable analysis recognizes that only the constructor can initialize class members and properly establish that certain fields will never be null at any point. It's a conundrum and the documentation and official examples and tutorials do often gloss this over (sometimes even with = null!, which I feel is just wrong).

  • Taking Blazor's ComponentBase for example (as this is what the OP's code is targeting, after-all): immediately after when the ComponentBase subclass is created (i.e. the ctor runs), the SetParametersAsync method is called, only after that then is OnInitializedAsync called - and OnInitializedAsync can fail or need to be awaited, which means that the "laws" about constructors definitely still apply: any code consuming a ComponentBase type cannot necessarily depend any late initialization by OnInitializedAsync to be guaranteed, especially not any error-handling code.

    • For example, if the ctor left the List<User> Users { get; } property uninitialized (and therefore null, despite the type not being List<User>?) and OnInitializedAsync (which would set it to a non-null value) were to fail, and then if that ComponentBase subclass object were to be passed to some custom-error handling logic, then that error-handler itself would fail if it (rightfully, but incorrectly) assumed that the Users property would never be null.
      • However this convoluted arrangement could have been avoided if Microsoft designed Blazor to support some kind of Component-factory system whereby the OnInitialized{Async} logic could be moved into a factory-method or ctor with that all-or-nothing guarantee that we need for software we can reason about. But anyway...
  • So given the above, there exist a few solutions:

    • If this were a system entirely under your control (which Blazor is not, so this advice doesn't apply to you) then I would recommend you redesign the system to use factory-methods instead of post-ctor object initialization.
    • But as this is Blazor, it shares certain similarities in its databinding approach with WPF, "XAML", WinForms and others: namely its roots in having long-lived mutable view-models implementing INotifyPropertyChanged.
      • And this is the most important factor: because the view-model concept means that you shouldn't have List<T> as the collection-type for a property that will be used for data-binding: you're meant to use ObservableCollection<T>, which solves the problem: ObservableCollection<T> is meant to be initialized only once by the constructor (thus satisfying the never-null problem), and is designed to be long-lived and mutable, so it's perfectly-fine to populate it in OnInitializedAsync and OnParametersSet{Async}, which is how Blazor operates.

Therefore, in my opinion, you should change your code to this:

class UsersListComponent : ComponentBase
{

    private Boolean isBusy;

    public Boolean IsBusy
    {
        get { return this.isBusy; }
        private set { if( this.isBusy != value ) { this.isBusy = value; this.RaiseOnNotifyPropertyChanged( nameof(this.IsBusy); } // INPC boilerplate, ugh: e.g. https://stackoverflow.com/questions/65813816/c-sharp-blazor-server-display-live-data-using-inotifypropertychanged
    }

    public ObservableCollection<User> Users { get; } = new ObservableCollection<User>();

    protected override async Task OnInitializedAsync()
    {
        await this.ReloadUsersAsync(); // Also call `ReloadUsersAsync` in `OnParametersSetAsync` as-appropriate based on your application.
    }

    private async Task ReloadUsersAsync()
    {
        this.IsBusy = true;
        this.Users.Clear(); // <-- The ObservableCollection will raise its own collection-modified events for you, which will be handled by the UI components data-bound to the exposed Users collection.

        try
        {
            List<User> users = await this.Context.Users
                .Include(u => u.Address)
                .ToListAsync();

            // ObservableCollection<T> doesn't support AddRange for historical reasons that we're now stuck with, ugh: https://github.com/dotnet/runtime/issues/18087#issuecomment-359197102

            foreach( User u in users ) this.Users.Add( u );
        }
        finally
        {
            this.IsBusy = false;
        }
    }
}

Notice how, if the data-loading in OnInitializeAsync, via ReloadUsersAsync, fails due to an exception being thrown from Entity Framework (which is common, e.g. SQL Server timeout, database down, etc) then the class-invariants of UsersListComponent (i.e. that the Users collection is never null and the property always exposes a single long-lived object-reference) always remain true, which means any code can safely consume your UsersListComponent without risk of a dreaded unexpected NullReferenceException.

(And the IsBusy part is just because it's an inevitable thing to add to any ViewModel/XAML-databound class that performs some IO).

Dai
  • 141,631
  • 28
  • 261
  • 374
  • This is super helpful. The explanation behind it especially so. – David Thielen Feb 11 '23 at 19:08
  • The practice of assigning null ! as the solution to this issue in Blazor is officially endorsed by the Blazor project (see https://github.com/dotnet/aspnetcore/issues/47736, answer posted by GitHub user javiercn). (I think "null !" is better than "default !" because it's more explicit about what is happening.) – Hammerite May 11 '23 at 13:38
  • @Hammerite I'm aware - there's also _criminal_ use of `= null!` in EF Core's documentation too. I feel that MS is advocating that out of pragmatic desire to address all the people more concerned with compiler-warnings they might not fully understand - and it takes far less effort to tell people to suppress warnings than it does to educate them in CS/OOP concepts when the same audience often lacks any formal CS education. Even so, that doesn't make it "right" when _demonstrably correct_ approaches exist (even when they do require a lot more fore-thought and understanding). – Dai May 11 '23 at 13:48
  • There are cases in which no "demonstrably correct" alternative exists. Your example code in this answer works only because there is a sensible non-null default value for a member of type ObservableCollection, namely an empty one. It is not uncommon to have a member of class type that has no such sensible default state, perhaps because it is heavyweight and must be constructed using the component's parameters, thus can only be constructed in OnInitialised[Async]() and not in the constructor. Your advice to categorically avoid assigning null ! is not practical in the circumstances. – Hammerite May 11 '23 at 15:23
  • @Hammerite "not practical" is entirely subjective - we're entitled to our own opinions :) – Dai May 11 '23 at 15:24