A functional-esque approach?

Background:

When designing with a functional mindset, object states tend to be immutable, with the object's design only exposing operations and data suitable for that particular state.

When objects are immutable, then their methods that put them into a new state would emit a new immutable object with those new operations available:

    class StateA
    {
        public String X { get; }

        public StateB GetNextState( String newData )
        {
            return new StateB( this, newData );
        }
    }

    class StateB
    {
        public StateB( StateA prevState, String newData )
        {
            this.X = prevState.X;
            this.Y = newData;
        }

        public String X { get; }
        public String Y { get; }
    }

This is as opposed-to the old-school of OOP design where objects were mutable (not just of their trivial data-members (getter/setter scalar properties) but also what they represent, where you might have this design:

    class State
    {
        public void NextState( String newData )
        {
            if( this.Y != null ) throw new InvalidOperationException( "Already in state B." );
            this.Y = newDate;
        }

        public String X { get; }
        public String Y { get; private set; }
    }

This State class is harder to reason about because the Y property exists and can be used by programs before NextState has been called, for example - and consumers can still call NextState() twice on the same object (granted, they'll get an exception by doing so) - whereas in the immutable example above, there isn't a GetNextState() method on StateB, which makes it impossible.

Now - thinking about how this applies to Entity Framework - I know the idea of only working with immutable entities is a bad idea - but at the same time I don't like how the same entity class is used to represent:

An entity that is "new" (and not yet inserted, without valid IDENTITY column values and database-computed values)
An entity that represents a valid and fully-populated database record intended to be mutated and Saved.
An entity that represents a valid and fully-populated database record intended only to be displayed or copied elsewhere and should not be modified (for fear of EF detecting unwanted changes when calling SaveChanges) - or when using AsNoTracking().
A "partial" entity being used for an UPDATE or DELETE operation.

In this case, we have 4 different states where the state objects should have different interfaces to prevent meaningless operations.

So consider a typical Orders and Products database:

    CREATE TABLE Products (
        ProductId int NOT NULL PRIMARY KEY IDENTITY(1,1),
        Name nvarchar(100) NOT NULL
    )

    CREATE TABLE Orders (
        OrderId int NOT NULL PRIMARY KEY IDENTITY(1,1),
    )

    CREATE TABLE OrderItems (
        OrderId int NOT NULL FOREIGN KEY REFERENCES Orders ( OrderId ),
        ProductId int NOT NULL FOREIGN KEY REFERENCES Products ( ProductId ),
        Qty int NOT NULL DEFAULT(1),
    )

Ordinarily in EF6 and EFCore, we'd get these classes (the keyword public omitted for brevity):

    class Product
    {
        Int32 ProductId { get; set; }
        String Name { get; set; }
        DateTime Created { get; set; }
        String Manufacturer { get; set; }
    }

    class Order
    {
        Int32 OrderId { get; set; }
        IList<OrderItem> OrderItems { get; set; }
    }

    class OrderItem
    {
        Int32 OrderId { get; set; }
        Int32 ProductId { get; set; }
        Int32 Qty { get; set; }
        Order Order { get; set; }
        Product Product { get; set; }
    }

But there are issues with these class designs:

The properties representing IDENTITY primary-key values are mutable, even though in most RDBMS, an IDENTITY column is immutable.
These classes are used by objects to represent entities that don't exist in the store yet, so they don't have a meaningful IDENTITY (or other database-generated value) value, but those properties are still exposed - which means their consumers need to know that if their value is zero or some other magic value (-1?) that they're "new" entities.
When using these entities as "partial entities" for UPDATE and DELETE, consumers have no way of knowing which properties are significant or not - which is why EF has the Entry<T> type which allows access to metadata about the state of each property member.
The classes are all fully mutable - which means if we intend to pass them off to untrusted consumers (e.g. ASP.NET MVC views) we must be careful to ensure they're queried with AsNoTracking() or that we don't call SaveChanges in the same DbContext.

I'd like to forward some ideas for making programming with EF entities "safer" which can be achieved simply by tweaking this T4 code-generator rather than having to change anything internal to EF - while still keeping entity types as mutable classes for most cases:

Proposal 1: Separate `New` and `Extant` entity subclasses:

Using the same Orders and Products database example as above, the code-generator could produce separate entity classes for "new" and "extant" entities.

A "new" entity class would not have any properties for values that are generated by the database - whereas an "extant" entity class does. Note that applications are still free to create a new instance of an "extant" entity object in-memory and then attach it to a DbContext to use as a "partial" entity as they can do today.

So we'd have:

    // Common mutable properties
    class CommonProduct
    {
        String Name { get; set; }
        DateTime Created { get; set; }
        String Manufacturer { get; set; }
    }

    // The "New" class is not a superclass or subclass of the main mutable entity, but is a cousin, to avoid inadvertent use of the wrong type by relying on languages' built-in covariance/contravariance handling
    class NewProduct : CommonProduct
    {
    }

    class Product : CommonProduct
    {
        Int32 ProductId { get; } // Immutable, as IDENTITY columns cannot be changed
    }

DbContext's DbSet<Product>.Add would then be modified like so:

    Task<Product> Add( NewProduct entity ) { ... }

Add only accepts NewProduct, so applications cannot accidentally try to add an extant Product.

Add returns a Task<Product> that resolves to the fully valid extant Product instance once SaveChanges or SaveChangesAsync is called, like so:

NewProduct newP = new NewProduct()
{
    Name = "Turboencabulator",
    Created = DateTime.UtcNow,
    Manufacturer = "General Electric"
};
Task<Product> pTask = dbContext.Products.Add( newP );
dbContext.SaveChanges();
Product p = pTask.Result;

(I appreciate this is more verbose than the current code, but I think the added safety makes it worthwhile)

Meanwhile the DbSet<Product>.Attach method remains the same:

    void Attach( Product entity ) { ... } // only accepts extant `Product` because its `ProductId` property must be valid.

Proposal 2A: Adding interfaces for read-only views of objects:

Following the example above, we add a new interface:

    interface IReadOnlyProduct
    {
        Int32 ProductId { get; }
        String Name { get; }
        DateTime Created { get; }
        String Manufacturer { get; }
    }

    class Product : CommonProduct, IReadOnlyProduct
    {
        Int32 ProductId { get; }
    }

Note that the interface is named IReadOnlyProduct because it provides a read-only view of a mutable object (so technically another thread could concurrently mutate the underlying Product object which would mess things up for the consumer of the IReadOnlyProduct - but it does mean that library authors don't need to worry about code accidentally mutating a mutable objects. It's also keeping with convention of other IReadOnly... types in the .NET standard libraries.

But as the mutable object is still returned, consumers could perform a valid cast and then mutate the object - but that's clearly intentional, whereas this post is more about preventing unintentional behaviour (i.e. bugs) through the use of appropriate types.

Proposal 2B: Truly immutable entity types:

A more stringent version of the above, this requires separate classes be defined (as immutable and mutable representations of the same aggregate cannot be derived from each other.

    class ImmutableProduct : IReadOnlyProduct
    {
        public ImmutableProduct( Int32 productId, String name, DateTime created, String manufacturer )
        {
            this.ProductId = productId;
            this.Name = name;
            this.Created = created;
            this.Manufacturer = manufacturer;
        }

        Int32 ProductId { get; }
        String Name { get; }
        DateTime Created { get; }
        String Manufacturer { get; }
    }

The EF DbContext's DbSet<T>'s AsNoTracking() extension method would be modified to return the immutable type (or just return IReadOnly... interfaces, which the immutable class implements anyway). Using AsNoTracking() has demonstrable performance benefits and returning an immutable object would help programmers understand why SaveChanges() doesn't work when changing the properties of AsNoTracking() entity objects.

Proposal 3: Read-only `DbContext` interface.

Somewhat related - I'd like it if the DbContext class only offered read-only operations at first, and that to perform operations that would be applied to the backing-store (i.e. SaveChanges/SaveChangesAsync) you would need a separate DbContext instance - or some kid of "unit-of-work" class.

I'm asking for this because when using ASP.NET's dependency-injection to inject a HTTP request-scoped DbContext instance, it's easy to forget how expensive SaveChanges calls are after pulling-in loads of entities but having only modified a few entities.

Something like this:

    class ProductsController : Controller
    {
        private readonly MyDbContext db;

        public ProductsController( MyDbContext db )
        {
            this.db = db;
        }

        [Route("/products")]
        public async Task<IActionResult> Index()
        {
            List<Product> products = await this.db.Products.ToListAsync();
            return this.View( products );
        }

        [HttpPost]
        public async Task<IActionResult> Index( EditProducts model )
        {
            List<Product> products = await this.db.Products.ToListAsync();

            if( this.ModelState.IsValid )
            {
                model.EditProducts( products );
                await this.db.SaveChanges();
            }

            return this.View( products );
        }
    }

Would be this instead:

    class ProductsController : Controller
    {
        private readonly ReadOnlyMyDbContext db;

        public ProductsController( ReadOnlyMyDbContext db )
        {
            this.db = db;
        }

        [Route("/products")]
        public async Task<IActionResult> Index()
        {
            // ReadOnlyMyDbContext applies `AsNoTracking()` to all queries by default, and returns all entities as immutable types or as read-only interfaces:
            List<IReadOnlyProduct> products = await this.db.Products.ToListAsync();
            return this.View( products );
        }

        [HttpPost]
        public async Task<IActionResult> Index( EditProducts model )
        {
            List<IReadOnlyProduct> products;
            if( this.ModelState.IsValid )
            {
                using( MyDbContext rwDb = this.db.ForMutable() )
                {
                    List<Product> rwProducts = rwDb.Products.ToListAsync();
                    model.EditProducts( rwProducts );
                    await rwDb.SaveChangesAsync();
                    products = rwProducts; // because `Product : IReadOnlyProduct`
                }
            }
            else
            {
                products = await this.db.Products.ToListAsync();
            }

            return this.View( products );
        }
    }

Proposal 4: Entity key structs:

Now this proposal is not concerned with any functional programming or concepts, but a little request that's been in my head for a while.

As C# does not support "strong" type-aliasing (only syntactical weak type-aliasing using This = That;) we need to define our own types.

Using a separate strong type for each entity's key means we can avoid bugs where entity keys are confused - for example:

    Int32 orderId = 123;
    Product product = db.Products.SingleOrDefault( p => o.ProductId == orderId );

I propose that each code-gen'd entity with a primary-key defined have a primary-key type defined and added to the entity type's interface.

    class ProductId : IEquatable<ProductId>, IEquatable<Int32>
    {
        public ProductId( Int32 productId )
        {
            if( productId < 1 ) throw new ArgumentOutOfRangeException( nameof(productId) );
            this.ProductId = productId;
        }

        public Int32 ProductId { get; }

        static operator explicit ProductId( Int32 value ) => new ProductId( value );
        static operator explicit Int32( ProductId value ) => value.ProductId;

        // implement IEquatable here
    }

or for composite keys:

    // supposing an Order has a composite primary-key of `CustomerId + OrderId`:

    class OrderId : IEquatable<OrderId>, IEquatable<(Int32,Int32)>
    {
        public OrderId( Int32 customerId, Int32 orderId )
        {
            if( customerId < 1 ) throw new ArgumentOutOfRangeException( nameof(customerId) );
            this.CustomerId = customerId;

            if( orderId < 1 ) throw new ArgumentOutOfRangeException( nameof(orderId ) );
            this.OrderId = orderId;
        }

        public Int32 CustomerId { get; }
        public Int32 OrderId { get; }

        static operator explicit OrderId( (Int32,Int32) value ) => new OrderId( value.Item1, value.Item2 );
        static operator explicit ( Int32, Int32 ) ( OrderId value ) => ( value.CustomerId, value.OrderId );

        // implement IEquatable here
    }

This would work with the earlier proposals where the immutable Order.OrderId property is now an instance of OrderId (the type) and the underlying Int32 values exposed only by also-immutable properties of OrderId.

...and the db.Products.SingleOrDefault( p => o.ProductId == orderId ); line would give a compiler error because class ProductId isn't comparable to class OrderId.

sjh37 / EntityFramework-Reverse-POCO-Code-First-Generator