3

In my database model I have a navigation property which cannot be null. However, since the variable may not have been loaded yet, it can be null during runtime.

public class Car {

    private Brand? _brand = null
    public Brand Brand {
        set => _brand = value;
        get => _brand ?? throw new InvalidOperationException("Uninitialized property: " + nameof(Brand ));
    }

    public string GetBrandLocation(){
        return this.Brand.Location;
    }

}

According to Microsoft documentation, this problem can be solved by throwing an exception in the getter. But all Exceptions that are thrown in the Getters are not catched by a global Exception handler. The execution of the code immediately stopps, after the exception has been thrown.

According to the Microsoft recommondations, throwing exceptions in Getters should be avoided. Also this Stackoverflow thread, advises against using Exceptions in Getters.

Another method would be, that I make the property also nullable. But if I do so, I have to check in each single function, if the property is null, which seems not to be very DRY.

public class Car {

    private Brand? _brand = null
    public Brand? Brand {
        set => _brand = value;
        get => _brand;
    }

    public string GetBrandLocation(){

        if(this.Brand == null){
            throw new InvalidOperationException("Uninitialized property: " + nameof(Brand ));
        }

        return this.Brand.Location;
        
    }

}

Because we have a big codebase, this can get really messy. Especially with chained calls like:

return Car.Brand.Adress.Street

Do I miss something? What is considered best practice in this use case?

M4SX5
  • 155
  • 1
  • 10
  • _"Do I miss something?"_ - yes: even as-of C# 10.0 (and 11?), the `#nullable` annotations are still insufficienctly expressive to cover _mutable_ objects like EF entity types. I don't know what the EFCore team specifically recommends, but in my case I use T4 tooling to generate interface-types and immutable classes to represent "Loaded..." entities with non-nullable navigation properties - though they are immutable and are not entity-types themselves. YMMV. – Dai Jul 18 '23 at 05:44

2 Answers2

1

If you normally try to access that objects property that is null, the compiler will throw an System.NullReferenceException that you can handle these errors globally. But if you want to throw your own exception and message, you must explicitly check when accessing the property, and in my opinion, the simplest and clean way is to use the Extension Methods as follows.

public static class Extensions
{
    public static T EnsureNotNull<T>(this T? type, string propertyName) where T : class
    {
        if (type == null)
        {
            throw new InvalidOperationException($"Uninitialized property: {propertyName}");
        }

        return type;
    }
}

and use as follows :

 return car.Brand.EnsureNotNull("brand").Adress.Street;
Mohammad Aghazadeh
  • 2,108
  • 3
  • 9
  • 20
  • ...how is this an improvement, though? you're still _running the risk_ of an unexpected exception: the fact it's `InvalidOperationException` instead of `NullReferenceException` is ultimately immaterial. However, different approaches (especailly those using class-invariants) mean you can _provably_ prevent exceptions entirely. – Dai Jul 18 '23 at 05:53
  • According to this part of the question ***Another method would be, that I make the property also nullable ....*** . Are you against throwing exceptions? Why is it unexpected? Exceptions are also a way to implement your logic. Also, can you guarantee that a property will never be null? So it is necessary to check it in every possible way – Mohammad Aghazadeh Jul 18 '23 at 06:05
  • _"Exceptions are also a way to implement your logic."_ - **I strongly disagree**: what you're describing is idiomatic to Java where exceptions are thrown-about all the time for "normal" error conditions, [but in C#/.NET exceptions must only be used for _exceptional_ things](https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions) - a maybe-null properties is not an exceptional thing. – Dai Jul 18 '23 at 06:12
  • "_Also, can you guarantee that a property will never be null?"_ - **Yes**, I can. That's what immutable-types and class-invariants (and constructors) are for. My answer demonstrates this. – Dai Jul 18 '23 at 06:13
  • _"Why is it unexpected?"_ - consider a function that could be anywhere in a project that accepts a `Car` parameter: that function _has no way of knowing_ if the `Car.Make` navigation property is loaded or not: supposing that the function's inner-logic requires the `Car.Make` property to be loaded but anyone can call the function and pass an invalid `Car` with a non-loaded `Make` property will cause it to unexpectedly crash _because_ C#/.NET does not currently allow a function to declare its preconditions. This is what the famed "Code Contracts" feature was all about, but it isn't in .NET Core+ – Dai Jul 18 '23 at 06:16
  • ***a maybe-null properties is not an exceptional thing*** Who said this? . it's completely dependent on the situation. And you cannot say that this is not an exception is **not an exceptional thing** – Mohammad Aghazadeh Jul 18 '23 at 06:58
  • We need to talk a lot about the things you said, but unfortunately,cannot be continued here – Mohammad Aghazadeh Jul 18 '23 at 07:04
1

In my database model I have a navigation property which cannot be null. However, since the variable may not have been loaded yet, it can be null during runtime.

Correct. EF Navigation properties will always need to be nullable, even if the underlying FK constraint (and NOT NULL column constraints) require a valid reference to always be present in the DB, because of the simple fact that loading related data is never required.

In that situation, the entity class properties corresponding to the FK columns will (of course) be non-nullable types, but any reference navigation properties must always be ? (i.e. nullable reference types). (Note: this does not apply to collection navigation properties, which are another story entirely).

According to Microsoft documentation, this problem can be solved by throwing an exception in the getter. But all Exceptions that are thrown in the Getters are not catched by a global Exception handler. The execution of the code immediately stopps, after the exception has been thrown.

I agree. I'm disappointed that EF's documentation is even suggesting that. This isn't anything new though: ever since they started seriously suggesting people do = null! to initialize DbContext properties (which, honestly, is just really, really dumb)

Another method would be, that I make the property also nullable. But if I do so, I have to check in each single function, if the property is null, which seems not to be very DRY.

Yes, but only if you use the `class Car`` EF type itself as a DTO for passing data around in your application.

...but if you instead design a new, separate immutable DTO class, with non-nullable properties, with a constructor that verifies these class-invariants, then that works great.

You can also use implicit conversions between the DTO and the entity-type to reduce some frictions, such as passing a DTO into a method expecting an entity, or to even use the DTOs with Linq Queries and DbSet and more.


So if this is your EF Entity class:

public class Car
{
    [Key]
    [DatabaseGenerated( Identity )]
    public Int32 CarId { get; set; } // CarId int NOT NULL IDENTITY PRIMARY KEY

    public Int32 MakeId { get; set; } // MakeId int NOT NULL CONSTRAINT FK_Car_Make FOREIGN KEY REFERENCES dbo.Makes ( MakeId )

    public Brand? Make { get; set; } // Navigation property for FK_Car_Make 
}

...and you want to represent a Car with a loaded Make propertty, then just add this to your project:

public class LoadedCarWithMake
{
    public LoadedCarWithMake( Car car, Make make )
    {
        this.Car  = car  ?? throw new ArgumentNullException(nameof(car));
        this.Make = make ?? throw new ArgumentNullException(nameof(make));

        // Ensure `make` corresponds to `car.Make`:
        if( !Object.ReferenceEquals( car.Make, make ) ) throw new ArgumentException( message: "Mismatched Car.Make", paramName: nameof(make) );
    }

    public Car  Car  { get; } // Immutable property, though `Car` is mutable.
    public Make Make { get; } // Immutable property, though `Make` is mutable.

    // Forward other members:
    public Int32 CarId  => this.Car.CarId;
    public Int32 MakeId => this.Car.MakeId;

    // Implicit conversion via reference-returns:
    public static implicit operator Car( LoadedCarWithMake self ) => self.Car;
}

Now, even if that Car entity has its Make navigation-property re-set to null or changed then that's okay because it won't affect consumers that use LoadedCarWithMake because LoadedCarWithMake.Make will never be null.

You'd also want to add a loader method for this too, e.g.:

public static async Task<LoadedCarWithMake> LoadCarWithMakeAsync( this MyDbContext db, Int32 carId )
{
    Car carWithMake = await db.Cars
        .Include( c => c.Make )
        .Where( c => c.CarId == carId )
        .SingleAsync()
        .ConfigureAwait(false);

    return new LoadedCarWithMake( car, car.Make! );
}

If all this extra repetitive code looks tedious, don't worry: you shouldn't normally need to write this by-hand: it's straightforward to use tools like T4 - or Roslyn Code Generation - to automatically create these "Loaded..." types for you - I just wish the EF team would include that in-box for the benefit of everyone.


You can improve this further by defining IReadOnly... interfaces for each entity-type (so you'd have IReadOnlyCar and IReadOnlyMake, which do not contain any navigation-properties, only get-only scalar/value properties), then LoadedCarWithMake would also then get to implement IReadOnlyCar.

Dai
  • 141,631
  • 28
  • 261
  • 374
  • Thank you for your detailed answer. It helps me a lot. If I understand your answer correctly, there is a separate "Loaded" class for each navigation property and for all combinations of it. If I now have 3 navigation properties, then that means that there must be 7 different "Loaded" classes, right? Doesn't that get very messy very quickly? – M4SX5 Jul 18 '23 at 06:34
  • @M4SX5 No, you only need 1 `Loaded...` class for each scenario: a `Loaded...` class can be used to assert that multiple navigation-properties are loaded, e.g. `LoadedCarWithMakeAndOwnerAndInsuranceInfo` for example (in practice you'd probably use a more succint name - and the good news is that using Roslyn Code Generators you can inspect each (non-dynamic) `IQueryable` for all the `.Include()` calls and use that for automatic codegen. – Dai Jul 19 '23 at 01:19