Efcore: Question: why do you not allow `IImmutableList<T>` for collection navigational properties with backing fields?

Created on 8 Jun 2020  路  4Comments  路  Source: dotnet/efcore

This is a question, rather than a request for a change!

I want to create entity classes where no properties can be changed outside the class (DDD-styled class). For instance:

```c#
public class Book
{
private readonly List _reviews;
public IEnumerable Reviews => _reviews.ToList();
//... other properties left out
}

From my testing I know that collection navigational properties can use the following types -

* `IEnumerable<T>`
* `IReadOnlyCollection<T>`

I want to move away from using  `IEnumerable<T>`, as it has (small) performance problems when you call it multiple times. I would like to use `IImmutableList<T>` as that sounds like a better type because it ensures that a) it takes a copy (`AsReadOnly` doesn't take a copy) and, b) makes parallel operations work better. But I get I use `IImmutableList<T>`.

TestSaveBookOneReviewAndReadOk [0:00.749] Failed: System.InvalidCastException : Unable to cast object of type 'System.Collections.Generic.HashSet1[DataLayer.EfClasses.Review]' to type 'System.Collections.Immutable.IImmutableList1[DataLayer.EfClasses.Review]'.
System.InvalidCastException : Unable to cast object of type 'System.Collections.Generic.HashSet1[DataLayer.EfClasses.Review]' to type 'System.Collections.Immutable.IImmutableList1[DataLayer.EfClasses.Review]'.
at lambda_method(Closure , InternalEntityEntry )
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.RelationshipsSnapshot..ctor(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.EnsureRelationshipSnapshot()
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntrySubscriber.SnapshotAndSubscribe(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges, Boolean modifyProperties)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable1 forceStateWhenUnknownKey) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode1 node)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraphTState
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
at Microsoft.EntityFrameworkCore.DbContext.SetEntityState(InternalEntityEntry entry, EntityState entityState)
at Microsoft.EntityFrameworkCore.DbContext.SetEntityStateTEntity
at Microsoft.EntityFrameworkCore.DbContext.AddTEntity
at Test.UnitTests.TestDataLayer.Ch08_RelationshipBackingFields.TestSaveBookOneReviewAndReadOk() in C:\Users\JonPSmith\source\repos\EfCoreinAction-SecondEdition\Test\UnitTests\TestDataLayer\Ch08_RelationshipBackingFields.cs:line 151
```

IImmutableList<T> seems to tick all the boxes by supporting IEnumerable<T>, ICollection<T>, IList<T>. Also AsReadOnly() doesn't work with a HashSet, but ToImmutableList() does. so why does EF Core not support it? Is there something fundamental or just the way it is written?

NOTE: I have tried this on EF Core 3.1 and 5-preview4

area-change-tracking area-c-mapping customer-reported type-enhancement

Most helpful comment

@JonPSmith It's worth keeping open--I suspect we can make this work.

All 4 comments

@JonPSmith I have been playing with this and I think we could make it work in a future version. It's more tricky than the other collection types because the backing field is not directly convertable to the property, and vice versa.

Hi @ajcvickers,

Thanks for looking at this. Totally happy to got with IReadOnlyCollection<T>, but just wondered why its that way.

For the DDD part of my book I'm going with

c# public class Book { private readonly IList<Review> _reviews; //Or HashSet if performance matters public IReadOnlyCollection<Review> Reviews => _reviews.ToImmutableList(); //... other properties left out }

I found that ToImmutableList() works for HashSet<T>, IList<T> etc. while AsReadOnly() doesn't work on either of those. I haven't checked said-by-side performance of ToImmutableList() and AsReadOnly() but I can't think its much (Doh! famous last works - I better check it)

I have left this open, but my question has been answered. Happy for you to close it if you want to.

@JonPSmith It's worth keeping open--I suspect we can make this work.

Timings of each, repeated to overcome first-use cost

1,000 x AsReadOnly took .14 ms., ave. per run = 135.6 ns.
1,000 x ToImmutableList took 2.60 ms., ave. per run = 2,596.2 ns.
1,000 x AsReadOnly took .00 ms., ave. per run = .9 ns.
1,000 x ToImmutableList took .08 ms., ave. per run = 81.0 ns.

So ToImmutableList is ~100 times slower but its so small I'm not sure it matters. I'll ponder on that.

Was this page helpful?
0 / 5 - 0 ratings