List<> causing Model to be flagged as IsDirty on BeginEdit()

Feb 18, 2015 at 2:31 PM
I have the following models:
    // ####################################################
    // CustomTab Model
    // ####################################################

    [XmlRoot("CustomTab")]
    public class CustomTab : ModelBase<CustomTab>
    {
        #region Initialization & Cleanup

        // Overriding the GetHashCode prevents the Clone operation from marking an 
        // object Dirty when it is first cloned
        // Calculates object has code based on property hash codes
        public override int GetHashCode()
        {
            return GetHashCode(this);
        }

        private int GetHashCode(object item, params string[] excludeProps)
        {
            int hashCode = 0;
            foreach (var prop in item.GetType().GetProperties())
            {
                if (!excludeProps.Contains(prop.Name))
                {
                    object propVal = prop.GetValue(item, null);
                    if (propVal != null)
                    {
                        hashCode = hashCode ^ propVal.GetHashCode();
                    }
                }
            }
            return hashCode;
        }


        public CustomTab()
        {
        }


        #endregion Initialization & Cleanup


        #region Properties

        [XmlElement("Header")]
        public String Header
        {
            get { return _Header; }
            set
            {
                _Header = value;
                NotifyPropertyChanged(m => m.Header);
            }
        }
        private String _Header = "";

        /// <summary>
        /// Sets the Visibility of the tab
        /// </summary>
        [XmlElement("TabVisibility")]
        public Visibility TabVisibility
        {
            get { return _TabVisibility; }
            set
            {
                _TabVisibility = value;
                NotifyPropertyChanged(m => m.TabVisibility);
            }
        }
        private Visibility _TabVisibility = Visibility.Visible;

        /// <summary>
        /// Collection of Tasks
        /// </summary>
        [XmlArray("Tasks")]
        [XmlArrayItem("Task", typeof(UtilitiesTask))]
        public List<UtilitiesTask> TaskCollection
        {
            get { return _TaskCollection; }
            set
            {
                _TaskCollection = value;
                NotifyPropertyChanged(m => m.TaskCollection);
            }
        }
        private List<UtilitiesTask> _TaskCollection;

                
        #endregion Properties

    }


    // ####################################################
    // UtilitiesTask Model
    // ####################################################
    public class UtilitiesTask : ModelBase<UtilitiesTask>   // : ModelBaseExtended, IIsDirty
    {
        #region Initialization & Cleanup

        // Overriding the GetHashCode prevents the Clone operation from marking an 
        // object Dirty when it is first cloned
        // Calculates object has code based on property hash codes
        public override int GetHashCode()
        {
            return GetHashCode(this);
        }

        private int GetHashCode(object item, params string[] excludeProps)
        {
            int hashCode = 0;
            foreach (var prop in item.GetType().GetProperties())
            {
                if (!excludeProps.Contains(prop.Name))
                {
                    object propVal = prop.GetValue(item, null);
                    if (propVal != null)
                    {
                        hashCode = hashCode ^ propVal.GetHashCode();
                    }
                }
            }
            return hashCode;
        }

        public UtilitiesTask()
        {
            
        }

        #endregion Initialization & Cleanup


        #region Properties

        [XmlElement("TaskTitle")]
        public String TaskTitle
        {
            get { return _TaskTitle; }
            set
            {
                _TaskTitle = value;
                NotifyPropertyChanged(m => m.TaskTitle);
            }
        }
        private String _TaskTitle = "";

        #endregion Properties
    }
}
I am using these models to create a VM as follows:
    public class CustomTabEditorViewModel : ViewModelDetailBase<CustomTabEditorViewModel, CustomTab>
    {

        // Default ctor
        public CustomTabEditorViewModel()
        {
            Log.Write(LogLevel.Debug, "Creating new CustomTabEditorViewModel with default constructor");
        }


        // One arguement ctor
        public CustomTabEditorViewModel(CustomTab customTab)
        {
            Log.Write(LogLevel.Debug, "Creating new CustomTabEditorViewModel with one arg constructor");
            base.Model = customTab;
            base.BeginEdit();
        }

        .....
    }
Stepping thru the BeginEdit() method I find that on Copy = Model.Clone(); is causing this to be flagged as dirty.

I have added the overriding GetHashCode() methods to each model but the VM is still being marked as IsDirty as soon as I call BeginEdit().

Can you tell me what I am doing wrong here? Removing the List<UtilitiesTask> from CustomTab results in the object NOT being marked as dirty so I assume there is something in the creation of the List.
Feb 19, 2015 at 7:31 AM
The way that IsDirty works is that a comparison takes place between an object and its clone, which is created when BeginEdit is called. The comparison is made between the result of GetHashCode on each object. If the hash codes are different, then the object is said to be "dirty," meaning that some change has been made to it.

If the object is a simple object, then you can make the comparison based on the result of OR-ing together the hash codes of all the properties. However, if the view model is of a list, then things are a bit more complicated. First, you need to exclude whatever properties you do not wish to be included in the comparison. For example, in your code, you would probably want to exclude all properties except TaskCollection, for example, things like Visibility, etc.

Second, you want to decide what it is that makes a list "dirty," for example, whether the two lists contain the same items, and whether any of those items is dirty. So you probably have to override GetHashCode in UtilitiesTask and test there to see if IsDirty is comparing objects the way you want to. Then you need to override GetHashCode in CustomTab by excluding all properties except TaskCollection, then return a hash code that is the result of OR-ing together the hash code of each UtilitiesTask in TaskCollection.

Does that make sense?

Cheers,
Tony
Feb 19, 2015 at 2:20 PM
What you outlined does make sense and that is what i had planned on doing.

However, I seem to be stuck with just getting the BeginEdit() to work even when ignoring the TaskCollection. As a test I eliminated the TaskParameterCollection from UtilitiesTask so that UtilitiesTask contains only basic objects, added "TaskCollection" to the GetHashCode ignore list and then stepped thru the code and recorded the HashCode value at various points (the following values were obtained using the Immediate Window in VS 2013):

Break point at Line 193 of ViewModelDetailBaseCore.cs (immediately after Original = Model;)
Model.GetHashCode() = -461136339
Original.GetHashCode() = -461136339

Stepping into private TModel Copy, break point at line 172 (prior to _copy = value;)
value.GetHashCode() = -461136339

At this point everything appears to have the same HashCode and is being compared correctly (ignoring "TaskCollection").

As soon as _copy = value; is executed (line 172 of ViewModelDetailBaseCore.cs) the object (this) is marked Dirty even though _copy.GetHashCode() = -461136339

I am unable to step into the _copy = value; statement so I am at a loss as to why the object is being marked dirty.

Thanks for you help with this, I really like using this framework!
Feb 19, 2015 at 2:33 PM
To find out what's going on, you will need to set a breakpoint in the IsDirty property of ViewModelDetailBaseCore.cs (line 292). That line calls the Copy.AreSame method, passing in a list of meta properties to exclude. As you can see, AreSame simply compares the hash codes of both the original and copy and returns true if they differ. That's the point at which you need to inspect the hash code values, which are represented by the variables hashCode1 and hashCode2 on lines 81 and 82 of ModelExtensions.cs. Let me know what you find out. Cheers!
Feb 19, 2015 at 2:49 PM
Edited Feb 19, 2015 at 2:49 PM
Thanks for the quick reply!

I believe the issue I am having is related to the GetObjectHashCode() method in ModelExtension.cs (line 69). This method is NOT calling the overriden GetHashCode() method from CustomTab and is not excluding the HashCodeValue of TaskCollection.

The list of excludeProps includes just meta properties and not those specified in the overriden GetHashCode().

I have also read that the HashCode value of a List, ObservableCollection, etc. is not from the objects within it but rather the instance of the object:

(from http://stackoverflow.com/questions/2907372/why-does-c-sharp-not-implement-gethashcode-for-collections)
Since collections are not immutable, they cannot implement GetHashCode.
Instead, they inherit the default GetHashCode, which returns a (hopefully) unique value for each instance of an object. (Typically based on a memory address)
Any ideas on how to get this to work or are collections just not going to work with ViewModelBaseDetail?
Feb 19, 2015 at 3:24 PM
GetObjectHashCode will call GetHashCode on each property of your view model. In your sample code, this would be CustomTabEditorViewModel. One of those should be your Model property, which I believe is of type CustomTab. You should be able to override GetHashCode in CustomTab to return a hash code based on the items in your list. The guidelines regarding immutability of GetHashCode pertain to when your objects are being used in a Dictionary, which is not the case here.

At the very least you should be able to set a breakpoint in CustomTab.GetHashCode and see execution break there. However, I would suggest you start by setting a breakpoint in the IsDirty property of ViewModelDetailBaseCore.cs. Have you tried that yet? From there you can step into GetHashCode to see what value is being calculated. Let me know if you can hit that breakpoint.
Feb 19, 2015 at 4:25 PM
Edited Feb 19, 2015 at 7:40 PM
I set a break point in ViewModelDetailBaseCore.IsDirty which in turn calls ModelExtension.AreSame(). It is within AreSame() that the hash codes for 'source' and 'item' are coming back as different. Both objects are of type CustomTab but when calculating these hash codes (using GetObjectHashCode()) the overriden GetHashCode() from CustomTab is NOT being used.


I think the issue is that AreSame is using GetObjectHashCode() which DOES NOT use the overriden GetHashCode() from CustomTab. If I call source.GetHashCode() and item.GetHashCode() from the Immediate window in VS both values come back with the same values and DO call the overriden GetHashCode() method in CustomTab. I even tried setting the return value from the overriden GetHashCode() to a constant (123) and I still have the same issue so I am pretty confident that the overriden GetHashCode() method is not being called here.


As I step thru the GetObjectHashCode() these are the values I am getting for GetHashCode() (line 79).

CustomTab 'source':
prop.Name           propVal         propVal.GetHashCode()           hashCode
'Header'              '555'               -461136340               -461136340
'TabIsVisible'        true                 1                       -461136339
'TaskCollection'      Count = 3**          45027976                -433288027
CustomTab 'item':
prop.Name           propVal           propVal.GetHashCode()        hashCode
'Header'              '555'               -461136340               -461136340
'TabIsVisible'        true                 1                       -461136339
'TaskCollection'      Count = 3**          45027976                -447291834
** A list of UtilityTask
Feb 19, 2015 at 4:49 PM
While debugging GetObjectHashCode, if you break on the call to propVal.GetHashCode(), what is the value of prop? Do you see the Model property on one of the iterations, and does prop.GetValue(item, null) return a value of type CustomTab which is not null?

If so, you can set a breakpoint on GetHashCode in CustomTab and see if you hit that breakpoint. If not, then there is something wrong with your override of GetHashCode in CustomTab.

Please let me know if you are able to hit these breakpoint. Thanks!
Feb 19, 2015 at 7:26 PM
The value of prop is like I outlined in my previous post ('555' - string, true - bool, TaskCollection - List<UtilityTask> with 3 items). Those are the only 3 props that make it to propVal.GetHashCode() on line 79.

The item passed into GetObjectHashCode() is my CustomTab model with the overriden GetHashCode() method but this overriden method is never called during GetObjectHashCode() as propVal at that point is a string, bool and List<>.
Feb 19, 2015 at 8:31 PM
As a test I created a new SimpleMVVM WPF application (using the template in VS) with the following additions/modifications:

Added CustomTab.cs to the Model directory:
    // ####################################################
    // CustomTab Model
    // ####################################################
    public class CustomTab : ModelBase<CustomTab>
    {
        #region Initialization & Cleanup

        // Overriding the GetHashCode prevents the Clone operation from marking an 
        // object Dirty when it is first cloned
        // Calculates object hash code based on property hash codes
        public override int GetHashCode()
        {
            return GetHashCode(this);  // , "TaskCollection");
        }

        private int GetHashCode(object item, params string[] excludeProps)
        {
            int hashCode = 0;
            foreach (var prop in item.GetType().GetProperties())
            {
                if (!excludeProps.Contains(prop.Name))
                {
                    object propVal = prop.GetValue(item, null);
                    if (propVal != null)
                    {
                        hashCode = hashCode ^ propVal.GetHashCode();
                    }
                }
            }
            return hashCode;
        }


        public CustomTab()
        {
        }


        #endregion Initialization & Cleanup


        #region Properties

        public String Header { get; set; }
        public Boolean TabIsVisible { get; set; }
        public List<UtilitiesTask> TaskCollection { get; set; }

        #endregion Properties

    }


    // ####################################################
    // UtilitiesTask Model
    // ####################################################
    public class UtilitiesTask : ModelBase<UtilitiesTask>   // : ModelBaseExtended, IIsDirty
    {
        #region Initialization & Cleanup

        // Overriding the GetHashCode prevents the Clone operation from marking an 
        // object Dirty when it is first cloned
        // Calculates object hash code based on property hash codes
        public override int GetHashCode()
        {
            return GetHashCode(this);
        }

        private int GetHashCode(object item, params string[] excludeProps)
        {
            int hashCode = 0;
            foreach (var prop in item.GetType().GetProperties())
            {
                if (!excludeProps.Contains(prop.Name))
                {
                    object propVal = prop.GetValue(item, null);
                    if (propVal != null)
                    {
                        hashCode = hashCode ^ propVal.GetHashCode();
                    }
                }
            }
            return hashCode;
        }

        public UtilitiesTask()
        {

        }

        #endregion Initialization & Cleanup


        #region Properties

        public String TaskTitle  { get; set; }
        public String ButtonLabel { get; set; }
        public String ButtonType { get; set; }

        #endregion Properties

    }
Modified MainPageViewModel.cs as follows:
    public class MainPageViewModel : ViewModelDetailBase<MainPageViewModel, CustomTab>
    {
        #region Initialization and Cleanup

        // Default ctor
        public MainPageViewModel() 
        {
            CustomTab _customTab = new CustomTab();
            _customTab.Header = "Test Tab";
            _customTab.TabIsVisible = true;
            _customTab.TaskCollection = new List<UtilitiesTask>();

            UtilitiesTask _task = new UtilitiesTask();
            _task.TaskTitle = "Task 1";
            _task.ButtonLabel = "Task 1 Button";
            _task.ButtonType = "SQL";

            _customTab.TaskCollection.Add(_task);

            _task = new UtilitiesTask();
            _task.TaskTitle = "Task 2";
            _task.ButtonLabel = "Task 2 Button";
            _task.ButtonType = "Crystal";

            _customTab.TaskCollection.Add(_task);

            CustomTabInfo = _customTab;

            this.Model = CustomTabInfo;
            base.BeginEdit();
        }

        #endregion

        #region Notifications

        // TODO: Add events to notify the view or obtain data from the view
        public event EventHandler<NotificationEventArgs<Exception>> ErrorNotice;

        #endregion

        #region Properties

        // Add properties using the mvvmprop code snippet
        public CustomTab CustomTabInfo { get; set; }

        private string bannerText = "Hello Simple MVVM Toolkit";
        public string BannerText
        {
            get
            {
                return "Banner";
            }
            set
            {
                bannerText = value;
                NotifyPropertyChanged(m => m.BannerText);
            }
        }

        #endregion
      
        . . . .
Using Snoop I can see that as soon as the program starts, the MainPageViewModel is marked as dirty. If I remove the List<> from the CustomTab object then MainPageViewModel is NOT marked as dirty.

Am I setting Model correctly and calling BeginEdit() correctly?
Feb 20, 2015 at 7:28 AM
The item passed into GetObjectHashCode() is my CustomTab model with the overriden GetHashCode() method but this overriden method is never called during GetObjectHashCode() as propVal at that point is a string, bool and List<>.
If the overriden GetHashCode isn't being called, then somehow it's not being properly implemented, and instead object.GetHashCode is being called, which will always return a code based on memory address and will cause the IsDirty switch to flip to true.

It's hard to tell from looking at pasted code what the problem is without stepping through it using a debugger. Can you send me a simple repro? I'm confident that stepping through the code will make clear what the problem is. Thanks!
Feb 20, 2015 at 12:40 PM
I have uploaded a simple repro here:

https://github.com/brianke/CustomTabTest.git

Thanks again for your time!
Feb 20, 2015 at 12:50 PM
No prob - I see the project. Can you list the steps needed to repro?
Feb 20, 2015 at 2:41 PM
Just run the program, CustomTabInfo in MainPageViewModel will be dirty after the call to BeginEdit(). I didn't put anything in the window that would show IsDirty, I was just using Snoops to look at the value. I can update it if you want.
Feb 20, 2015 at 6:55 PM
Edited Feb 23, 2015 at 6:21 PM
I will take a look over the weekend ... Looking now ....
Feb 24, 2015 at 7:43 AM
I've taken a closer look and have a bit of information for you now. First, there is an issue with the latest version of the toolkit on NuGet (v5.5.0.1), where the Json Serializer is not happy with the [DataContract] attribute on ModelBaseCore. I'm working on a fix for that. But in the meantime, just use the 5.5 version.

The other piece of info is that GetHashCode is not called on Model, but on each property in the model. The issue comes down to the List<T> property. What you need to do is wrap that in a class in which you do override GetHashCode.

More info to follow ...
Feb 24, 2015 at 1:19 PM
Thanks for looking into this.

Do you by chance have any examples of deserializing XML into a List wrapped inside a Class? I am having trouble getting the XML to deserialize into the List. I added the following to my CustomTab code:
    [XmlRoot("CustomTab")]
    public class CustomTab : ModelBase<CustomTab>
    {
        . . . .
        [XmlElement("Tasks")]
        public TaskCollection TaskCollection { get; set; }
    }

    // ####################################################
    // TaskCollection Model
    // ####################################################
    public class TaskCollection : ModelBase<TaskCollection>
    {
        #region Initialization & Cleanup
        // Overriding the GetHashCode prevents the Clone operation from marking an 
        // object Dirty when it is first cloned
        // Calculates object hash code based on property hash codes
        public override int GetHashCode()
        {
            return GetHashCode(this, "TaskList");
        }

        private int GetHashCode(object item, params string[] excludeProps)
        {
            int hashCode = 0;
            foreach (var prop in item.GetType().GetProperties())
            {
                if (!excludeProps.Contains(prop.Name))
                {
                    object propVal = prop.GetValue(item, null);
                    if (propVal != null)
                    {
                        hashCode = hashCode ^ propVal.GetHashCode();
                    }
                }
            }
            return hashCode;
        }


        public TaskCollection()
        {
            TaskList = new List<UtilitiesTask>();
        }

        #endregion Initialization & Cleanup

        [XmlArrayItem("Task", typeof(UtilitiesTask))]
        public List<UtilitiesTask> TaskList { get; set; }
    }
This now properly leaves IsDirty as false but the property TaskCollection is not populated with the XML <Tasks>.
Feb 24, 2015 at 1:27 PM
Edited Feb 24, 2015 at 1:36 PM
Hang on, I'm just about to push v5.5.1 to NuGet. Then I'll send you a GitHub pull request with the code that will work for you. No need for XML serialization -- the toolkit is now using Json for serialization anyways. The code for the custom collection will be quite simple, but give me just a bit to post it here and send you the PR. :)

PS. v5.5.1 is now on NuGet.org, so it's available to use. I updated the code that uses Json.Net to make it happy with the DataContract attribute -- by adding a JsonObject attribute.

I'm now working on a PR for you with the code you need.
Feb 24, 2015 at 2:15 PM
Alright, I submitted a pull request for your GitHub repo with the updated code. I cleaned up the repo a bit by adding a gitignore and getting rid of the binary files. It depends on v5.5.1 of the toolkit NuGet package, which you can update from NuGet.org.

Here is the TaskCollection.cs file which I added and enables correct functioning of IsDirty with collections.
public class TaskCollection : ObservableCollection<UtilitiesTask>
{
    public override int GetHashCode()
    {
        int hashCode = 0;
        foreach (var item in Items)
        {
            if (item != null)
            {
                // Aggregate item hash codes
                hashCode = hashCode ^ item.GetHashCode(); 
            }
        }
        return hashCode;
    }
}
Cheers,
Tony
Feb 25, 2015 at 6:35 PM
Edited Feb 25, 2015 at 9:28 PM
Thanks for all your help. I have updated my project as you have shown and the XML parsing is working and the object is NOT dirty when first calling BeginEdit().

I do however have one new problem. I am trying to bind to the IsDirty property and the binding is not being updated with the correct IsDirty value. I have created a new Git project, IsDirtyTest (https://github.com/brianke/IsDirtyTest.git). I create a CustomTab and display its values on the screen. If you change a value, the IsDirty property is correctly changed to True (when viewed with Snoops) but the binding to IsDirty is not updating. Can you tell me if I am binding to IsDirty incorrectly?
Feb 26, 2015 at 9:15 AM
To solve this new problem, I submitted a pull request. The key is a method called AssociateProperties, which you need to call after BeginEdit in your ctor so that changes to the Header property of your model fire a PropertyChangedEvent on the view model for the IsDirty property, causing the binding to pick it up.

Notice the code I inserted but commented out, in order to show the steps I took to debug the problem. I added a wrapper IsDirtyInfo property to be able to check that the binding was reading the property after the PropertyChanged event was fired. I also added a button to fire the event manually. And I also had to remove the binding in the MainPage xaml to the MainPageViewModel.

GitHub is a great way to collaborate on issues. :) In the future, be sure to add a .gitignore file such as the one I included with my PR. Then the bin and obj folders will be excluded from your repo, making it much easier for others to use.

Here is the call to AssociateProperties. I don't have a method which associates all the model properties, so you'll have to add a call to AssociateProperties for each model property.
// Associate Header with IsDirty
AssociateProperties(m => m.Header, vm => vm.IsDirty);
Cheers,
Tony
Feb 26, 2015 at 12:49 PM
Edited Feb 26, 2015 at 12:49 PM
Thanks again Tony. I think the problem boils down to not having specified the NotifyPropertyChanged calls in each of the model properties (Header, TabIsVisible, etc.). I removed all your changes EXCEPT those with NotifyPropertyChanged on the model properties and it now works as expected. I think in my effort to reduce any extraneous code I must have removed those.

I have put the working code back up on GitHub (thanks for adding the ignore, I am brand new to GitHub and still trying to learn it). If the AssociateProperties is the preferred way of accomplishing what I need (as opposed to adding NotifyPropertyChange to each property) please let me know.
Feb 26, 2015 at 12:56 PM
Welcome to the world of GitHub! :) Eventually I'll move the source code for this project over there, where I currently have another project of mine called Trackable Entities, which enables n-tier support for Entity Framework.

I believe you'll need to have both the property setters with NotifyPropertyChanged and the call to AssociateProperties, if you want to have a change in your model properties trigger the PropertyChanged event for IsDirty in your view model. Let me know if you think that's not necessary.
Feb 26, 2015 at 1:22 PM
I updated the IsDirtyTest project, removing the AssociateProperties and adding the NotifyPropertyChanged to each CustomTab property and everything appears to work correctly so I don't think AssociateProperty is required (as long as NotifyPropertyChanged is called).
Feb 26, 2015 at 1:35 PM
Can you push your changes back to GitHub so I can check them out?
Feb 26, 2015 at 1:45 PM
The changes are in GitHub already (at least I think so :) I can see the changes online under the Master branch)
Feb 26, 2015 at 8:13 PM
Edited Feb 27, 2015 at 4:24 AM
Tony, I have one more issue with this IsDirty flag. I implemented a ListView to display a collection of CustomTabs and I am back to the issue where the IsDirty flag is correctly set in code when something changes but the notification is not being sent.

I have committed one more change to GitHub (Implement ListView of CustomTabs). I created a new model of CustomTabInfo with two properties, ConfigModelName and CustomTabCollection. Changes to ConfigModelName do propagate the IsDirty flag (the text on screen changes to true) but changes to CustomTabCollection do not.

I tried your various debug methods but I could not see where the issue was. If I am understanding this correctly the issue lies in the fact that CustomTabCollection : ObservableCollectionExtended<CustomTab> does not implement NotifyPropertyChanged but I am not sure how to implement this on the ObservableCollection.


Again I really appreciate all your help with this!
Feb 27, 2015 at 8:53 AM
I submitted another pull request with the answer to your question. Basically it involves overriding Insert and Remove methods on the custom collection in order to subscribe to property changes on each item.

Cheers,
Tony
Feb 27, 2015 at 12:31 PM
Edited Feb 27, 2015 at 12:34 PM
Excellent, works like a charm. Thanks for all your help!

I will leave this project up on GitHub should anyone else come across this post.
Feb 27, 2015 at 12:42 PM
Glad to help!