ValeraT1982 / ObjectsComparer

C# Framework provides mechanism to compare complex objects, allows to override comparison rules for specific properties and types.
MIT License
352 stars 86 forks source link

Comparision of object lists in Expando objects only looks at the count #29

Closed ChrEggenberger closed 3 years ago

ChrEggenberger commented 3 years ago

Comparing an ExpandoObject with a property with a value of a string List works fine.

        [Test]
        public void StringListFields()
        {
            dynamic a1 = new ExpandoObject();
            a1.Field1 = new List<string>() { "A", "B" };
            dynamic a2 = new ExpandoObject();
            a2.Field1 = new List<string>() { "A", "C" };
            var comparer = new Comparer();

            var isEqual = comparer.Compare(a1, a2, out IEnumerable<Difference> differencesEnum);

            Assert.IsFalse(isEqual);
        }

But the same thing with a object list fails

     [Test]
        public void ObjectListsFields()
        {
            dynamic a1 = new ExpandoObject();
            a1.Field1 = new List<object>() { "A", "B" };
            dynamic a2 = new ExpandoObject();
            a2.Field1 = new List<object>() { "A", "C" };
            var comparer = new Comparer();

            var isEqual = comparer.Compare(a1, a2, out IEnumerable<Difference> differencesEnum);

            Assert.IsFalse(isEqual);
        }

We encountered the problem using your good library together with deserializing an ExpandoObject from a json. which creates this generic object list.

ValeraT1982 commented 3 years ago

This is an expected behavior. If type of the list is object Comparer compares all properties/fields of a class object. As far as object class doesn't have any properties or fields Comparer consider elements of the list equal

ChrEggenberger commented 3 years ago

Yes, I understand your point. Your library is using a comparer which suits the type of the property. In our case it makes more sense to use the type of the property value whenever possible. This can be different (see my sample). I do not know if this would make sense in a general way as well and I'm not proud of my change to your code at all nor do I know for sure if it works for all cases. Especially because it changes the behavior in somewhat unpredictable way. And furthermore I only cover the case where obj1 and obj2 are of the same type. It would probably make sense to return a difference if they are not of the same type. I added in the public class Comparer<T> : AbstractComparer<T> :

        public override IEnumerable<Difference> CalculateDifferences(T obj1, T obj2)
        {
            var type = typeof(T);
            if (_members.Count == 0 && !type.IsArray
                && obj1 != null && obj2 != null
                && obj1.GetType().Equals(obj2.GetType()))
            {
                var targetType = obj1.GetType();
                if (!type.Equals(targetType)
                    && !(targetType.IsArray || targetType.Namespace != null && targetType.Namespace.StartsWith("System.Collections")))
                {
                    var targetTypeComparer = Factory.GetObjectsComparer(targetType, Settings, this);
                    return targetTypeComparer.CalculateDifferences(targetType, obj1, obj2);
                }
            }
            return CalculateDifferences(obj1, obj2, null);
        }
ValeraT1982 commented 3 years ago

I agree that logic you implemented may make sense for some scenarios. You can copy source code in your project and make this change or you can implement custom comparer with factory.