Improvements to value comparer docs

Including docs for new value comparer fluent API

entity-framework/core/modeling/value-comparers.md

@@ -16,43 +16,34 @@ uid: core/modeling/value-comparers
 
 ## Background
 
-EF Core needs to compare property values when:
+In EF, change tracking means that EF automatically determines what changes were performed by the application on a loaded entity instance, so that those changes can be saved back to the database when <xref:Microsoft.EntityFrameworkCore.DbContext.SaveChanges%2A> is called. EF usually performs this by taking a *snapshot* of the instance when it's loaded from the database, and *comparing* that snapshot to the instance handed out to the application.
 
-* Determining whether a property has been changed as part of [detecting changes for updates](xref:core/saving/basic)
-* Determining whether two key values are the same when resolving relationships
+EF comes with built-in logic for snapshotting and comparing most standard types used in databases, so users don't usually need to worry about this topic. However, when a property is mapped through a [value converter](xref:core/modeling/value-conversions), EF Core needs to perform comparison on arbitrary user types, which may be complex. By default, EF uses the default equality comparison defined by types (e.g. the `Equals` method); for snapshotting, value types are copied to produce the snapshot, while for reference types no copying occurs, and the same instance is used as the snapshot.
 
-This is handled automatically for common primitive types such as int, bool, DateTime, etc.
+In cases where the built-in comparison behavior isn't appropriate, users may provide a *value comparer*, which contains logic for snapshotting, comparing and calculating a hash code. For example, the following sets up value conversion for `List<int>` property to be value converted to a JSON string in the database, and defines an appropriate value comparer as well:
 
-For more complex types, choices need to be made as to how to do the comparison.
-For example, a byte array could be compared:
+[!code-csharp[ListProperty](../../../samples/core/Modeling/ValueConversions/MappingListProperty.cs?name=ConfigureListProperty)]
 
-* By reference, such that a difference is only detected if a new byte array is used
-* By deep comparison, such that mutation of the bytes in the array is detected
+See [mutable classes](#mutable-classes) below for further details.
+
+Note that value comparers are also used when determining whether two key values are the same when resolving relationships; this is explained below.
 
-By default, EF Core uses the first of these approaches for non-key byte arrays.
-That is, only references are compared and a change is detected only when an existing byte array is replaced with a new one.
-This is a pragmatic decision that avoids deep comparison of many large byte arrays when executing SaveChanges.
-But the common scenario of replacing, say, an image with a different image is handled in a performant way.
+## Shallow vs. deep comparison
 
-On the other hand, reference equality would not work when byte arrays are used to represent binary keys.
-It's very unlikely that an FK property is set to the _same instance_ as a PK property to which it needs to be compared.
-Therefore, EF Core uses deep comparisons for byte arrays acting as keys.
-This is unlikely to have a big performance hit since binary keys are usually short.
+For small, immutable value types such as `int`, EF's default logic works quite well: the value is copied as-is when snapshotted, and compared with the type's built-in equality comparison. When implementing your own value comparer, it's important to consider whether deep or shallow comparison (and snapshotting) logic is appropriate.
 
-### Snapshots
+Consider byte arrays, which can be arbitrarily large. These could be compared:
 
-Deep comparisons on mutable types means that EF Core needs the ability to create a deep "snapshot" of the property value.
-Just copying the reference instead would result in mutating both the current value and the snapshot, since they are _the same object_.
-Therefore, when deep comparisons are used on mutable types, deep snapshotting is also required.
+* By reference, such that a difference is only detected if a new byte array is used
+* By deep comparison, such that mutation of the bytes in the array is detected
 
-## Properties with value converters
+By default, EF Core uses the first of these approaches for non-key byte arrays. That is, only references are compared and a change is detected only when an existing byte array is replaced with a new one. This is a pragmatic decision that avoids copying entire arrays and comparing them byte-to-byte when executing <xref:Microsoft.EntityFrameworkCore.DbContext.SaveChanges%2A>, and the common scenario of replacing, say, one image with another is handled in a performant way.
 
-In the case above, EF Core has native mapping support for byte arrays and so can automatically choose appropriate defaults.
-However, if the property is mapped through a [value converter](xref:core/modeling/value-conversions), then EF Core can't always determine the appropriate comparison to use.
-Instead, EF Core always uses the default equality comparison defined by the type of the property.
-This is often correct, but may need to be overridden when mapping more complex types.
+On the other hand, reference equality would not work when byte arrays are used to represent binary keys, since it's very unlikely that an FK property is set to the _same instance_ as a PK property to which it needs to be compared. Therefore, EF Core uses deep comparisons for byte arrays acting as keys; this is unlikely to have a big performance hit since binary keys are usually short.
 
-### Simple immutable classes
+Note that the chosen comparison and snapshotting logic must correspond to each other: deep comparison requires deep snapshotting to function correctly.
+
+## Simple immutable classes
 
 Consider a property that uses a value converter to map a simple, immutable class.
 
@@ -67,7 +58,7 @@ Properties of this type do not need special comparisons or snapshots because:
 
 So in this case the default behavior of EF Core is fine as it is.
 
-### Simple immutable Structs
+## Simple immutable structs
 
 The mapping for simple structs is also simple and requires no special comparers or snapshotting.
 
@@ -77,10 +68,10 @@ The mapping for simple structs is also simple and requires no special comparers
 
 EF Core has built-in support for generating compiled, memberwise comparisons of struct properties.
 This means structs don't need to have equality overridden for EF, but you may still choose to do this for [other reasons](/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type).
-Also, special snapshotting is not needed since structs immutable and are always memberwise copied anyway.
+Also, special snapshotting is not needed since structs are immutable and are always copied memberwise anyway.
 (This is also true for mutable structs, but [mutable structs should in general be avoided](/dotnet/csharp/write-safe-efficient-code).)
 
-### Mutable classes
+## Mutable classes
 
 It is recommended that you use immutable types (classes or structs) with value converters when possible.
 This is usually more efficient and has cleaner semantics than using a mutable type.
@@ -90,24 +81,24 @@ For example, mapping a property containing a list of numbers:
 
 [!code-csharp[ListProperty](../../../samples/core/Modeling/ValueConversions/MappingListProperty.cs?name=ListProperty)]
 
-The [`List<T>` class](/dotnet/api/system.collections.generic.list-1):
+The <xref:System.Collections.Generic.List%601> class:
 
 * Has reference equality; two lists containing the same values are treated as different.
 * Is mutable; values in the list can be added and removed.
 
 A typical value conversion on a list property might convert the list to and from JSON:
 
-[!code-csharp[ConfigureListProperty](../../../samples/core/Modeling/ValueConversions/MappingListProperty.cs?name=ConfigureListProperty)]
+### [EF Core 5.0](#tab/ef5)
 
-This then requires setting a `ValueComparer<T>` on the property to force EF Core use correct comparisons with this conversion:
+[!code-csharp[ListProperty](../../../samples/core/Modeling/ValueConversions/MappingListProperty.cs?name=ConfigureListProperty&highlight=7-10)]
 
-[!code-csharp[ConfigureListPropertyComparer](../../../samples/core/Modeling/ValueConversions/MappingListProperty.cs?name=ConfigureListPropertyComparer)]
+### [Older versions](#tab/older-versions)
 
-> [!NOTE]  
-> The model builder ("fluent") API to set a value comparer has not yet been implemented.
-> Instead, the code above calls SetValueComparer on the lower-level IMutableProperty exposed by the builder as 'Metadata'.
+[!code-csharp[ListProperty](../../../samples/core/Modeling/ValueConversions/MappingListPropertyOld.cs?name=ConfigureListProperty&highlight=8-11,17)]
 
-The `ValueComparer<T>` constructor accepts three expressions:
+***
+
+The <xref:Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer%601> constructor accepts three expressions:
 
 * An expression for checking equality
 * An expression for generating a hash code
@@ -119,7 +110,7 @@ Likewise, the hash code is built from this same sequence.
 (Note that this is a hash code over mutable values and hence can [cause problems](https://ericlippert.com/2011/02/28/guidelines-and-rules-for-gethashcode/).
 Be immutable instead if you can.)
 
-The snapshot is created by cloning the list with ToList.
+The snapshot is created by cloning the list with `ToList`.
 Again, this is only needed if the lists are going to be mutated.
 Be immutable instead if you can.
 
@@ -129,18 +120,17 @@ Be immutable instead if you can.
 > Conceptually, this is similar to compiler inlining.
 > For example, a simple conversion may just be a compiled in cast, rather than a call to another method to do the conversion.
 
-### Key comparers
+## Key comparers
 
 The background section covers why key comparisons may require special semantics.
 Make sure to create a comparer that is appropriate for keys when setting it on a primary, principal, or foreign key property.
 
-Use [SetKeyValueComparer](/dotnet/api/microsoft.entityframeworkcore.mutablepropertyextensions.setkeyvaluecomparer) in the rare cases where different semantics is required on the same property.
+Use <xref:Microsoft.EntityFrameworkCore.MutablePropertyExtensions.SetKeyValueComparer%2A> in the rare cases where different semantics is required on the same property.
 
-> [!NOTE]  
-> SetStructuralComparer has been obsoleted in EF Core 5.0.
-> Use SetKeyValueComparer instead.
+> [!NOTE]
+> <xref:Microsoft.EntityFrameworkCore.MutablePropertyExtensions.SetStructuralValueComparer%2A> has been obsoleted in EF Core 5.0. Use <xref:Microsoft.EntityFrameworkCore.MutablePropertyExtensions.SetKeyValueComparer%2A> instead.
 
-### Overriding defaults
+## Overriding the default comparer
 
 Sometimes the default comparison used by EF Core may not be appropriate.
 For example, mutation of byte arrays is not, by default, detected in EF Core.

samples/core/Modeling/ValueConversions/MappingImmutableClassProperty.cs

@@ -89,7 +89,7 @@ public override bool Equals(object obj)
                 => ReferenceEquals(this, obj) || obj is ImmutableClass other && Equals(other);
 
             public override int GetHashCode()
-                => Value;
+                => Value.GetHashCode();
         }
         #endregion
     }

samples/core/Modeling/ValueConversions/MappingListProperty.cs

@@ -21,14 +21,14 @@ public void Run()
 
                 ConsoleWriteLines("Save a new entity...");
 
-                var entity = new EntityType { MyProperty = new List<int> { 1, 2, 3 } };
+                var entity = new EntityType { MyListProperty = new List<int> { 1, 2, 3 } };
                 context.Add(entity);
                 context.SaveChanges();
 
                 ConsoleWriteLines("Mutate the property value and save again...");
 
                 // This will be detected and EF will update the database on SaveChanges
-                entity.MyProperty.Add(4);
+                entity.MyListProperty.Add(4);
 
                 context.SaveChanges();
             }
@@ -39,7 +39,7 @@ public void Run()
 
                 var entity = context.Set<EntityType>().Single();
 
-                Debug.Assert(entity.MyProperty.SequenceEqual(new List<int> { 1, 2, 3, 4 }));
+                Debug.Assert(entity.MyListProperty.SequenceEqual(new List<int> { 1, 2, 3, 4 }));
             }
 
             ConsoleWriteLines("Sample finished.");
@@ -55,23 +55,14 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                 #region ConfigureListProperty
                 modelBuilder
                     .Entity<EntityType>()
-                    .Property(e => e.MyProperty)
+                    .Property(e => e.MyListProperty)
                     .HasConversion(
                         v => JsonSerializer.Serialize(v, null),
-                        v => JsonSerializer.Deserialize<List<int>>(v, null));
-                #endregion
-
-                #region ConfigureListPropertyComparer
-                var valueComparer = new ValueComparer<List<int>>(
-                    (c1, c2) => c1.SequenceEqual(c2),
-                    c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
-                    c => c.ToList());
-
-                modelBuilder
-                    .Entity<EntityType>()
-                    .Property(e => e.MyProperty)
-                    .Metadata
-                    .SetValueComparer(valueComparer);
+                        v => JsonSerializer.Deserialize<List<int>>(v, null),
+                        new ValueComparer<List<int>>(
+                            (c1, c2) => c1.SequenceEqual(c2),
+                            c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
+                            c => c.ToList()));
                 #endregion
             }
 
@@ -87,7 +78,7 @@ public class EntityType
             public int Id { get; set; }
 
             #region ListProperty
-            public List<int> MyProperty { get; set; }
+            public List<int> MyListProperty { get; set; }
             #endregion
         }
     }

samples/core/Modeling/ValueConversions/MappingListPropertyOld.cs

@@ -0,0 +1,92 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Text.Json;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.ChangeTracking;
+using Microsoft.Extensions.Logging;
+
+namespace EFModeling.ValueConversions
+{
+    public class MappingListPropertyOld : Program
+    {
+        public void Run()
+        {
+            ConsoleWriteLines("Sample showing value conversions for a List<int>...");
+
+            using (var context = new SampleDbContext())
+            {
+                CleanDatabase(context);
+
+                ConsoleWriteLines("Save a new entity...");
+
+                var entity = new EntityType { MyListProperty = new List<int> { 1, 2, 3 } };
+                context.Add(entity);
+                context.SaveChanges();
+
+                ConsoleWriteLines("Mutate the property value and save again...");
+
+                // This will be detected and EF will update the database on SaveChanges
+                entity.MyListProperty.Add(4);
+
+                context.SaveChanges();
+            }
+
+            using (var context = new SampleDbContext())
+            {
+                ConsoleWriteLines("Read the entity back...");
+
+                var entity = context.Set<EntityType>().Single();
+
+                Debug.Assert(entity.MyListProperty.SequenceEqual(new List<int> { 1, 2, 3, 4 }));
+            }
+
+            ConsoleWriteLines("Sample finished.");
+        }
+
+        public class SampleDbContext : DbContext
+        {
+            private static readonly ILoggerFactory
+                Logger = LoggerFactory.Create(x => x.AddConsole()); //.SetMinimumLevel(LogLevel.Debug));
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder)
+            {
+                #region ConfigureListProperty
+                modelBuilder
+                    .Entity<EntityType>()
+                    .Property(e => e.MyListProperty)
+                    .HasConversion(
+                        v => JsonSerializer.Serialize(v, null),
+                        v => JsonSerializer.Deserialize<List<int>>(v, null));
+
+                var valueComparer = new ValueComparer<List<int>>(
+                    (c1, c2) => c1.SequenceEqual(c2),
+                    c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
+                    c => c.ToList());
+
+                modelBuilder
+                    .Entity<EntityType>()
+                    .Property(e => e.MyListProperty)
+                    .Metadata
+                    .SetValueComparer(valueComparer);
+                #endregion
+            }
+
+            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
+                => optionsBuilder
+                    .UseLoggerFactory(Logger)
+                    .UseSqlite("DataSource=test.db")
+                    .EnableSensitiveDataLogging();
+        }
+
+        public class EntityType
+        {
+            public int Id { get; set; }
+
+            #region ListProperty
+            public List<int> MyListProperty { get; set; }
+            #endregion
+        }
+    }
+}

samples/core/Modeling/ValueConversions/Program.cs

@@ -13,6 +13,7 @@ public static void Main()
             new MappingImmutableClassProperty().Run();
             new MappingImmutableStructProperty().Run();
             new MappingListProperty().Run();
+            new MappingListPropertyOld().Run();
             new OverridingByteArrayComparisons().Run();
         }
 

samples/core/Modeling/ValueConversions/ValueConversions.csproj

@@ -8,8 +8,8 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="3.1.8" />
-        <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="3.1.8" />
+        <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="5.0.0" />
+        <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="5.0.0" />
     </ItemGroup>
 
 </Project>