Fix a number of code smells

I'm not going to treat the change of SyncRoot from
a field to a property as a breaking change.  Technically
it is, but it's a mistake that it was a field anyway.

Author: craigfowler

CSF.Entities/Entity`1.cs

@@ -74,7 +74,7 @@ object IEntity.IdentityValue
         /// </summary>
         /// <returns>The converted value.</returns>
         /// <param name="value">The value to convert.</param>
-        TIdentity ToIdentityType(object value)
+        static TIdentity ToIdentityType(object value)
         {
             var formatter = System.Globalization.CultureInfo.InvariantCulture;
             var converted = Convert.ChangeType(value, typeof(TIdentity), formatter);

CSF.Entities/IdentityFactory.cs

@@ -33,12 +33,12 @@ public class IdentityFactory : ICreatesIdentity
     {
         static readonly IGetsIdentityType identityTypeProvider = new IdentityTypeProvider();
 
-        /// <summary>
-        /// Create an identity from the specified parameters.
-        /// </summary>
-        /// <returns>The created identity, or a <c>null</c> reference if the identity value is equal to the default of its data-type.</returns>
-        /// <param name="entityType">The entity type.</param>
-        /// <param name="identityValue">The identity value.</param>
+        /// <summary>
+        /// Create an identity from the specified parameters.
+        /// </summary>
+        /// <returns>The created identity, or a <c>null</c> reference if the identity value is equal to the default of its data-type.</returns>
+        /// <param name="entityType">The entity type.</param>
+        /// <param name="identityValue">The identity value.</param>
         public IIdentity Create(Type entityType, object identityValue)
         {
             if (entityType == null)
@@ -79,7 +79,7 @@ public IIdentity Create(Type entityType, Type identityType, object identityValue
             return (IIdentity)Activator.CreateInstance(closedIdentityType, new[] { convertedValue });
         }
 
-        object GetDefaultValue(Type identityType)
+        static object GetDefaultValue(Type identityType)
             => identityType.IsValueType ? Activator.CreateInstance(identityType) : null;
     }
 }

CSF.ORM.Entities/EntityData.cs

@@ -162,21 +162,17 @@ public void Update<TEntity>(TEntity entity) where TEntity : class, IEntity
             if (entity == null)
                 throw new ArgumentNullException(nameof(entity));
 
-            return AddAsyncInternal(entity, token);
+            return AddInternalAsync(entity, token);
         }
 
-        async Task<IIdentity<TEntity>> AddAsyncInternal<TEntity>(TEntity entity, CancellationToken token = default(CancellationToken)) where TEntity : class, IEntity
+        async Task<IIdentity<TEntity>> AddInternalAsync<TEntity>(TEntity entity, CancellationToken token = default(CancellationToken)) where TEntity : class, IEntity
         {
-            var identity = entity.GetIdentity();
-            object idValue;
-            if (identity != null)
-            {
-                idValue = await persister.AddAsync(entity, identity.Value, token);
-                return ReferenceEquals(idValue, null) ? null : (IIdentity<TEntity>)idFactory.Create(typeof(TEntity), idValue);
-            }
+            var currentIdentity = entity.GetIdentity();
+            var currentIdentityValue = (currentIdentity != null)? currentIdentity.Value : null;
 
-            idValue = await persister.AddAsync(entity, null, token);
-            return ReferenceEquals(idValue, null) ? null : (IIdentity<TEntity>)idFactory.Create(typeof(TEntity), idValue);
+            var newIdentityValue = await persister.AddAsync(entity, currentIdentityValue, token)
+                .ConfigureAwait(false);
+            return (newIdentityValue is null) ? null : (IIdentity<TEntity>) idFactory.Create(typeof(TEntity), newIdentityValue);
         }
 
         /// <summary>

CSF.ORM.Entities/IdentityGeneratingPersisterDecorator.cs

@@ -94,15 +94,17 @@ public void Delete<T>(T item, object identity) where T : class
         public async Task<object> AddAsync<T>(T item, object identity = null, CancellationToken token = default(CancellationToken)) where T : class
         {
             if (identity != null)
-                return await wrapped.AddAsync<T>(item, identity, token);
+                return await wrapped.AddAsync<T>(item, identity, token).ConfigureAwait(false);
 
             if (item is IEntity entity)
             {
                 identityGenerator.UpdateWithIdentity(entity);
-                return await wrapped.AddAsync<T>((T)entity, entity.IdentityValue, token);
+                return await wrapped.AddAsync<T>((T)entity, entity.IdentityValue, token)
+                    .ConfigureAwait(false);
             }
 
-            return await wrapped.AddAsync<T>(item, identity, token);
+            return await wrapped.AddAsync<T>(item, identity, token)
+                .ConfigureAwait(false);
         }
 
         /// <summary>

CSF.ORM.Entities/IdentityPopulatingTheoryQueryDecorator.cs

@@ -91,7 +91,8 @@ public TQueried Theorise<TQueried>(object identityValue) where TQueried : class
         /// <typeparam name="TQueried">The type of object to retrieve.</typeparam>
         public async Task<TQueried> TheoriseAsync<TQueried>(object identityValue, CancellationToken token = default(CancellationToken)) where TQueried : class
         {
-            var output = await wrapped.TheoriseAsync<TQueried>(identityValue, token);
+            var output = await wrapped.TheoriseAsync<TQueried>(identityValue, token)
+                .ConfigureAwait(false);
 
             if (output is IEntity entity && !entity.HasIdentity)
                 entity.IdentityValue = identityValue;

CSF.ORM.NHibernate.Common/EagerFetchingProvider.cs

@@ -129,7 +129,7 @@ public IQueryableWithEagerFetching<TQueried, TGrandchild> FetchGrandchildren<TQu
             return new QueryableWithFetchingAdapter<TQueried, TGrandchild>(nhFetchRequest);
         }
 
-        IQueryable<T> GetUnderlyingQueryIfAvailable<T>(IQueryable<T> query)
+        static IQueryable<T> GetUnderlyingQueryIfAvailable<T>(IQueryable<T> query)
             => (query is IProvidesQueryable<T> queryProvider) ? queryProvider.GetQueryable() : query;
 
         INhFetchRequest<TQueried, TFetched> GetNhFetchRequest<TQueried, TFetched>(IQueryable<TQueried> query)

CSF.ORM/InMemory/DataStore.cs

@@ -36,7 +36,7 @@ public class DataStore
         /// <summary>
         /// Gets a synchronisation object under which the current instance may be locked.
         /// </summary>
-        public readonly ReaderWriterLockSlim SyncRoot;
+        public ReaderWriterLockSlim SyncRoot { get; }
 
         /// <summary>
         /// Gets the collection of items in the data store.

PersistenceTester/CSF.PersistenceTester.Core/Impl/PersistenceTester.cs

@@ -1,5 +1,5 @@
 using System;
-using CSF.EqualityRules;
+using CSF.EqualityRules;
 using CSF.ORM;
 
 namespace CSF.PersistenceTester.Impl
@@ -47,8 +47,8 @@ PersistenceTestResult TrySetup()
                 };
             }
             finally
-            {
-                conn?.Dispose();
+            {
+                conn?.Dispose();
             }
 
             return null;
@@ -77,8 +77,8 @@ PersistenceTestResult TrySave()
                 };
             }
             finally
-            {
-                conn?.Dispose();
+            {
+                conn?.Dispose();
             }
 
             return null;
@@ -113,8 +113,8 @@ PersistenceTestResult TryCompare()
                 };
             }
             finally
-            {
-                conn?.Dispose();
+            {
+                conn?.Dispose();
             }
         }
 
@@ -124,22 +124,22 @@ PersistenceTestResult TryCompare()
         /// This method ensures that both values can be converted to string now, while we still have the connection.
         /// </summary>
         /// <param name="equalityResult">An equality result.</param>
-        void EnsureEqualityResultMayBeReportedUpon(EqualityResult equalityResult)
-        {
-            foreach(var item in equalityResult.RuleResults)
+        static void EnsureEqualityResultMayBeReportedUpon(EqualityResult equalityResult)
+        {
+            foreach(var item in equalityResult.RuleResults)
             {
                 try { item.ValueA?.ToString(); }
                 catch(Exception e)
-                {
-                    throw new PersistenceTestingException($"An exception was raised when pre-converting the persisted value {item.Name} to string.", e);
-                }
-
+                {
+                    throw new PersistenceTestingException($"An exception was raised when pre-converting the persisted value {item.Name} to string.", e);
+                }
+
                 try { item.ValueB?.ToString(); }
                 catch (Exception e)
                 {
                     throw new PersistenceTestingException($"An exception was raised when pre-converting the retrieved value {item.Name} to string.", e);
                 }
-            }
+            }
         }
 
         /// <summary>

PersistenceTester/CSF.PersistenceTester.NUnit/Constraints/NotEqualResult.cs

@@ -26,12 +26,12 @@ void WriteFailingRule(MessageWriter writer, EqualityRuleResult ruleResult)
                 writer.WriteLine($"{RuleName(ruleResult)} Expected {Format(ruleResult.ValueA)} but got {Format(ruleResult.ValueB)}");
         }
 
-        string RuleName(EqualityRuleResult ruleResult)
+        static string RuleName(EqualityRuleResult ruleResult)
         {
             return $">>> [{ruleResult.Name,20}]:";
         }
 
-        string Format(object value)
+        static string Format(object value)
         {
             if (ReferenceEquals(value, null)) return "<null>";
             if (value is string || value is char) return $"\"{value}\"";

PersistenceTester/CSF.PersistenceTester.NUnit/Constraints/SuccessfulPersistenceConstraint.cs

@@ -8,10 +8,10 @@ namespace CSF.PersistenceTester.Constraints
     public class SuccessfulPersistenceConstraint : Constraint
     {
         /// <summary>
-        /// Applies the constraint to an actual value, returning a ConstraintResult.
-        /// </summary>
-        /// <param name="actual">The value to be tested</param>
-        /// <returns>A ConstraintResult</returns>
+        /// Applies the constraint to an actual value, returning a ConstraintResult.
+        /// </summary>
+        /// <param name="actual">The value to be tested</param>
+        /// <returns>A ConstraintResult</returns>
         public override ConstraintResult ApplyTo<TActual>(TActual actual)
         {
             ConstraintResult result;
@@ -35,15 +35,15 @@ public override ConstraintResult ApplyTo<TActual>(TActual actual)
             return new NotEqualResult(this, testResult.EqualityResult);
         }
 
-        ConstraintResult GetTypeCheckResult<TActual>(TActual actual)
+        static ConstraintResult GetTypeCheckResult<TActual>(TActual actual)
         {
             var constraint = new InstanceOfTypeConstraint(typeof(PersistenceTestResult));
             var result = constraint.ApplyTo(actual);
             if (result?.IsSuccess == true) return null;
             return result;
         }
 
-        ConstraintResult GetNullCheckResult(PersistenceTestResult actual)
+        static ConstraintResult GetNullCheckResult(PersistenceTestResult actual)
         {
             var constraint = new NotConstraint(new NullConstraint());
             var result = constraint.ApplyTo(actual);