Regression somewhere between release & trunk

Jan 4, 2010 at 7:53 PM

I upgraded to the trunk to get the fix you mentioned in a previous thread.  Unfortunately, now some of our unit tests that were previously working are now failing.  We aren't getting collection change events when we use the following code:

            // This first line is here to provide the basic idea and may not compile.
            Expression<Func<TItem, TDerived>> getDerivedExpression = p => p.Value;

            Func<TItem, IValueProvider<TDerived>> getDervived = ExpressionObserver.Compile(getDerivedExpression);
            Expression<Func<TItem, Tuple<TItem, TDerived>>> getPairExpression = item => new Tuple<TItem, TDerived>(item, getDervived(item).Value);
            Func<TItem, IValueProvider<Tuple<TItem, TDerived>>> getPair = ExpressionObserver.Compile(getPairExpression);
            return Obtics.Collections.ObservableEnumerable.Select(list, getPair);

Any ideas?

Jan 4, 2010 at 8:14 PM

It looks like the regression has something to do with object equality.  I've included a test that fails on the new obtics, but when the Equals region is commented out, it works again.

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Linq;
using System.Linq.Expressions;
using System.Text;
using NUnit.Framework;
using Obtics.Values;

namespace Pleasant.Watchers.Tests
{
    [TestFixture]
    public class ObticsTests
    {
        [Test]
        public void TupleCollection()
        {
            var innerList = new ObservableCollection<TestObject>();

            innerList.Add(new TestObject(3));
            innerList.Add(new TestObject(5));
            innerList.Add(new TestObject(1));

            var tupleCollection = GetTupleCollection(innerList, o => o.Value);

            int changeCount = 0;
            (tupleCollection as INotifyCollectionChanged).CollectionChanged += delegate { changeCount++; };

            innerList[1].Value = 9;

            Assert.AreEqual(1, changeCount);
        }

        /// <summary>
        /// Returns an observable collection of Tuples.  Value1 in the tuple will be the list item.
        /// Value2 in the tuple will be the result of the expression passed in.
        /// </summary>
        /// <typeparam name="TItem"></typeparam>
        /// <typeparam name="TDerived"></typeparam>
        /// <param name="getDerivedExpression"></param>
        /// <returns></returns>
        public static IEnumerable<Tuple<TItem, TDerived>> GetTupleCollection<TItem, TDerived>(IEnumerable<TItem> list, Expression<Func<TItem, TDerived>> getDerivedExpression)
        {
            Func<TItem, IValueProvider<TDerived>> getDervived = ExpressionObserver.Compile(getDerivedExpression);
            Expression<Func<TItem, Tuple<TItem, TDerived>>> getPairExpression = item => new Tuple<TItem, TDerived>(item, getDervived(item).Value);
            Func<TItem, IValueProvider<Tuple<TItem, TDerived>>> getPair = ExpressionObserver.Compile(getPairExpression);
            return Obtics.Collections.ObservableEnumerable.Select(list, getPair);
        }

        public class Tuple<T1, T2>
        {
            public T1 Value1 { get; private set; }
            public T2 Value2 { get; private set; }

            public Tuple(T1 value1, T2 value2)
            {
                Value1 = value1;
                Value2 = value2;
            }

            public override bool Equals(object obj)
            {
                if (obj == null)
                    return false;

                if (GetType() != obj.GetType())
                    return false;

                Tuple<T1, T2> tuple = (Tuple<T1, T2>)obj;

                return
                    EqualityComparer<T1>.Default.Equals(Value1, tuple.Value1) &&
                    EqualityComparer<T2>.Default.Equals(Value2, tuple.Value2);
            }

            public override int GetHashCode()
            {
                int hashCode = 0;
                if (Value1 != null) hashCode ^= Value1.GetHashCode();
                if (Value2 != null) hashCode ^= Value2.GetHashCode();
                return hashCode;
            }
        }

        public class TestObject : INotifyPropertyChanged
        {
            public TestObject(int value)
            {
                Value = value;
            }

            public int Value
            {
                get
                {
                    return _Value;
                }
                set
                {
                    _Value = value;
                    NotifyPropertyChanged("Value");
                }
            }
            private int _Value;

            #region ************************* THIS SECTION BREAKS OBTICS **************************

            public override bool Equals(object obj)
            {
                TestObject other = obj as TestObject;
                if (other == null) return false;

                return
                    Value == other.Value;
            }

            public override int GetHashCode()
            {
                return Value.GetHashCode();
            }

            #endregion

            public event PropertyChangedEventHandler PropertyChanged;

            protected void NotifyPropertyChanged(string propertyName)
            {
                if (PropertyChanged != null)
                    PropertyChanged.Invoke(this, new PropertyChangedEventArgs(propertyName));
            }
        }
    }
}

Coordinator
Jan 9, 2010 at 10:30 AM

Hi,

It could very well be that the problem has to do with the equality overrides. The way you implement equality in your TestObject makes it not 'well-behaved' (as described here http://obtics.codeplex.com/wikipage?title=Well-behaved).

The point is that as soon as you change the value of your Value property the identity of your TestObject changes. Not only Obtics but anything that uses a hashtable will have difficulties with it. You can try placing a TestObject in a HashSet, change its Value and then try to retrieve it again. Obtics uses hashtables extensivly for memoization. With a carrousel it tries to keep down the total allocates size for transformation pipelines. 

Many textbooks will say that the way you implemented equality here is fine and proper. These books are wrong! The implementation is not strict enough. The way it is implemented in your Tuple class is correct. In this case the identity never changes because you can't change the values of Value1 and Value2 (provided the values themselves don't change identity as TestObject does.).

That this worked before must have been purely by chance. Proper implementation of equality and functions that always return an equal result when given equal parameters is very important when working with Obtics.

In case you realy need equality implemented the way you have then there could be a solution in the ObticsEqualityComparerAttribute. With this you can specify an alternative EqualityComparer for obtics to use. This will only work though when Obtics manipulates your object directly. See also http://obtics.codeplex.com/WorkItem/View.aspx?WorkItemId=5603, http://obtics.codeplex.com/WorkItem/View.aspx?WorkItemId=5208

Hope this helps,

Thomas.