0

I have a small architectural problem with MVVM. In this question I will note Model through M and ViewModel through VM.

I have 4 collections that must be synchronized, and the items in them must have their properties synchronized. The collection synchronizing part is done. But now, I wonder if I have to change this architecture for easier maintenance in future because I have to change properties' values of a VM when an indirectly linked M changes the value of some of its properties.

Maybe I have to store in each VM a link to a corresponding M, but it is not always possible because I sync VMs to Ms, these Ms to other VMs, and these last VMs to other Ms, and some information is lost on this way.

In the figure below bools are boolean flags that indicate whether synching in that direction is allowed or blocked. I block synching of a collection with another collection to prevent infinite recursivity.

scheme

The classes behind the collections are not fixed, I can do any refactoring as necessary and I did not made them very well from the start.

The 4 classes are:

  • ClockGroupM inherits from BindableBase (INotifyPropertyChanged implementation in Prism)
    • props:
      • Name (of type string)
      • IsSelected (bool)
      • Icon (Bitmap)
  • ClockGroupVM inherits from ClockGroupM
    • props:
      • DisplayString (string)
      • Foreground (Brush)
      • FontStyle (FontStyle)
  • FilterM
    • props:
      • GroupNames (List<string>)
      • SearchString (string)
      • ShowActive (bool)
      • ShowInactive (bool)
      • ShowTimers (bool)
      • ShowAlarms (bool)
      • Clocks (reference to the corresponding ClockMCollection on which the filter is applied)
      • Groups (reference to the corresponding ClockGroupMCollection)
      • Autocorrected (bool, indicates whether there was coercion of the search string in the user interface)
    • has a Combine method that combines two filters into one and
    • just a c-tor that accepts a reference to a ClockMCollection and a string (the search string)
  • FilterVM
    • props:
      • MyFilter (FilterM)
      • DisplayString (string)
      • MyConstantImageSource (ImageSource)
      • MyEmptyImageSource (ImageSource)
      • MyNonEmptyImageSource (ImageSource)
      • MyImageSource (ImageSource)
      • IsSelected (bool)
      • Items (int)
      • HasContent (bool)
    • method: UpdateItemCount (updates the Items counter property, returns void)

The 4 collections are the following (they are not directly the ObservableCollection instances, because I created wrappers for the ObservableCollections for more flexibility, and in each VMCollection there is a reference to the corresponding MCollection):

  • FilterMCollection - represent search filters, there are a few basic filters that work with multiple groups and that should not be deletable (e.g. Active, Inactive etc.)
  • FilterVMCollection - the filters' VMs as shown in the "Filters" panel
  • ClockGroupVMCollection - the groups' VMs as shown for example in the menu used to change the appartenance of an item to a group, so basic filters are excluded from this
  • ClockGroupMCollection - the groups as stored in the data file

The synching is always done through a single static method with many parameters. I use C#, WPF and Visual Studio.

I am a beginner in MVVM and this is the first question of mine on this site after a long period of time.

Thank you for patience, for any help, advice or suggestion.

Update 1

My application is a clock manager (a clock can be a timer or an alarm, in future maybe a stopwatch too). The clocks can be part of a group (similar to a folder). The clocks can be filtered. There are 5 base filters: All, Active, Inactive, Timers, Alarms. In future I think there will be custom saved filters based on search strings.

The M suffix stands for Model, and VM stands for ViewModel.

I can provide any piece of code needed from my application source, but please excuse my too big comments. Currently my application is not open source but it will be. Here is the method that does the synchronization:

internal static void SynchronizeCollectionChange<T, U>(
    ObservableCollection<T> source,
    NotifyCollectionChangedEventArgs e,
    ObservableCollection<U> target,
    Action<U> afterAddition = null,
    Action<T> prepareDeletion = null,
    Func<T, U, bool> equalsWithinTargetTo = null,
    Func<T, U> toTarget = null,
    int startingIndexInSource = 0,
    int startingIndexInTarget = 0)
{
    switch (e.Action)
    {
        case NotifyCollectionChangedAction.Add:
            foreach (T item in e.NewItems)
            {
                var mItem = (T)item;
                int idx = source.IndexOf(mItem);
                // if it doesn't exist in target, put it in target at idx, not with Add method
                //int uIdx = target.IndexOf(toTarget(mItem));
                //int newIndex = idx;
                //if (startingIndexInTarget != -1) // only from this index will the addition take place
                //{
                //    if (idx <startingIndexInTarget) // if the index in sourceis lower than the starting index in source that you  take care of
                //    {
                //        continue; // go to next added item, ignore this one
                //    }
                //    //// else
                //    //newIndex = startingIndexInSource + idx;
                //}
                //else
                //{
                //    newIndex = idx;
                //}
                // if the index in target is... it does not matter... the new index is computed by adding starting
                //if(uIdx < startingIndexInTarget)
                //{
                //}
                // the operation could have been either Insert or Add
                //if (0 <= uIdx && uIdx <= source.Count - 1)
                //{
                //    target.Insert(newIndex, toTarget(mItem)); // move
                //}
                //else
                //{
                int ni = idx - startingIndexInSource + startingIndexInTarget;
                if (ni < 0 || ni > target.Count)
                {
                    // the item must not be copied to target, it is a basic (auto) filter
                    break;
                }
                    target.Insert(ni, toTarget(mItem)); // add
                    //target.Add(toTarget(mItem));
                    afterAddition?.Invoke(toTarget(mItem));
                //}
            }
            break;
        case NotifyCollectionChangedAction.Remove:
            foreach (T item in e.OldItems)
            {
                // here the T item is already deleted from source
                // this should cvt T item to an U , but not when deleting it!
                //U mItem = toTarget(item); // does not know that and that
                //if (mItem == null)
                //{
                //    return;
                //}
                prepareDeletion?.Invoke(item);
                // here the T item is already deleted from source
                // find VM objects that wrap the relevant model object and remove them
                IEnumerable<U> query;
                while ((query = from it in target
                                where equalsWithinTargetTo(item, it) // based on fact that U : T
                                select it).Count() > 0)
                {
                    U m = query.First();
                    //if (startingIndexInTarget != -1)
                    //{
                    //    if (target.IndexOf(m) < startingIndexInTarget)
                    //    {
                    //        continue;
                    //    }
                    //}
                    //int index = target.IndexOf(m);
                    target.Remove(x => ReferenceEquals(x, m));
                }
            }
            break;
        case NotifyCollectionChangedAction.Reset:
            if (startingIndexInTarget != 0 || startingIndexInSource != 0)
            {
                for (int i = target.Count - 1; i >= 0; --i)
                {
                    int ni = i - startingIndexInSource + startingIndexInTarget;
                    if (ni < 0 || ni >= target.Count)
                    {
                        // the item must not be cleared from target on reset, it is a basic (auto) filter
                        continue;
                    }
                    target.RemoveAt(i);
                }
            }
            else
            {
                target.Clear();
            }
            break;
        case NotifyCollectionChangedAction.Move:
            {
                //if (startingIndexInTarget != -1 &&
                //    (e.OldStartingIndex < startingIndexInTarget ||
                //    e.NewStartingIndex < startingIndexInTarget))
                //{
                //    break;
                //}
                // TODO: handle multiple items
                int oi = e.OldStartingIndex - startingIndexInSource + startingIndexInTarget;
                int ni = e.NewStartingIndex - startingIndexInSource + startingIndexInTarget;
                if (ni < 0 || ni > target.Count)
                {
                    break;
                }
                if (oi < 0 || oi > target.Count)
                {
                    break;
                }
                target.Move(oi, ni);
                break;
            }
        case NotifyCollectionChangedAction.Replace:
            {
                //if (startingIndexInTarget != -1 &&
                //    (e.OldStartingIndex < startingIndexInTarget ||
                //    e.NewStartingIndex < startingIndexInTarget))
                //{
                //    break;
                //}
                int oi = e.OldStartingIndex - startingIndexInSource + startingIndexInTarget;
                if (oi < 0 || oi > target.Count)
                {
                    break;
                }
                // TODO: handle multiple items
                target[oi] = toTarget((T)e.NewItems[0]);
                break;
            }
        default:
            throw new NotImplementedException();
    }
}

Here can be found the synching routines using the method above (some were omitted because they are in another part of the source not in the discussion, I think).

I will refactor to make ClockGroupM not inherit from BindableBase. Now I understand that the VM should be a BindableBase, not the M. And I should have a method in the VM that updates its properties with values from the M, these properties will keep the Views updated through data binding.

Why I do not synchronize just the models and model collections: I am not very sure... I think it is because:

  1. I create new models and remove old models and I do not know how to notify the collection of VMs about this and let it create/remove corresponding VMs.

  2. Another reason is that I need to be notified when a M instance changes one of its properties, this is why I inherit from BindableBase.

  3. Another situation I faced was that a VM changed its inner M reference because simply a clock changed its type (i.e. from timer it became an alarm) and the VM had to handle this:

internal void ChangeTypeOfClock(ClockM cd, Type newType)
{
    Model.PropertyChanged -= Model_PropertyChanged;

    if (newType == typeof(TimerData))
    {
        if (cd.IsActive)
        {
            cd.ActivateOrDeactivate();
        }

        var tdata = new TimerData(Model.MyDataFile, Model.MyDataFile.MultiAudioPlayer);

        tdata.GroupName = cd.GroupName;
        tdata.Icon = cd.Icon;
        tdata.Style = cd.Style;
        tdata.UserBackColor = cd.UserBackColor;
        tdata.Tag = cd.Tag;
        tdata.IsUnsaved = true;
        tdata.ID = cd.ID;
        tdata.Checkable = cd.Checkable;
        tdata.Checked = cd.Checked;

        Model = tdata;
    }
    else // AlarmData
    {
        if (cd.IsActive)
        {
            cd.ActivateOrDeactivate();
        }

        var adata = new AlarmData(Model.MyDataFile, Model.MyDataFile.MultiAudioPlayer);

        adata.GroupName = cd.GroupName;
        adata.Icon = cd.Icon;
        adata.Style = cd.Style;
        adata.UserBackColor = cd.UserBackColor;
        adata.Tag = cd.Tag;
        adata.IsUnsaved = true;
        adata.ID = cd.ID;
        adata.Checkable = cd.Checkable;
        adata.Checked = cd.Checked;

        Model = adata;
    }

    Model.PropertyChanged += Model_PropertyChanged;

    RaisePropertyChanged("CurrentValue");
    RaisePropertyChanged("ResetToValue");
    RaisePropertyChanged("IsActive");
    RaisePropertyChanged("IsUnsaved");

    ClockTypeChanged?.Invoke(this, new ClockTypeChangedEventArgs()
    {
        ClockVM = this,
        NewClockM = Model,
        OldClockM = cd
    });

    IsUnsaved = true;
}

In one week I would like to publish a new version of my application.

Thank you very much.

Update 2

I posted the 8 classes here - they are somehow functional but the arrows in the figure above only synchronize items in collections, not the properties of existing items. After that version I started refactoring, and the code is currently not working because I still have questions:

  1. If I use two-way data binding with the VM, the best thing to do is to inherit from BindableBase in the VM class, or to use manually made synchronization methods (SyncFromModel, SyncToModel), for both directions, or both?

  2. If an M changes the value of some of its properties, how does the VM know that it changed? Changes are coming not from the UI but from the database in the background... For example, when the M of a group of clocks changes its ItemsCount property how do I send this notification to the View? Currently I send it from the M to the ClockGroupMCollection, then from it I update the ClockMCollection.IsUnsaved property to have the value true.

  3. What would be the MVVM way to change the model: from the VM, from the database code? How do I access the models from the DB code? I need a collection of models for this, right?

  4. Should I have just collectons of VMs, or collections of Ms too? I need to have a collection of clock group models because it is that collection that is traversed to apply a filter to the list of clocks that is visible to the user in the UI. Is this a case where synching is better than the alternative?

If it helps to say this, I also have in another part of the application a VM that stores the edited-in-UI value corresponding to a value in the model that was previously saved or loaded from a database. I think I have to use a bound command that requests all the edited values from all the VMs and saves them to the database. Currently I do this without commands but it works well. In this are implicated these classes:

  • SettingDataM
  • SettingDataMCollection
  • SettingDataVM and its subclasses:
    • AudioFileSettingDataVM
    • CheckBoxSettingDataVM
    • ImageFileSettingDataVM
    • IntegerSettingDataVM
    • TimeSpanSettingDataVM

I use these in the Settings window of the application.

I also have these classes:

  • ClockM (with subclasses AlarmData and TimerData)
  • ClockVM
  • ClockMCollection
  • ClockVMCollection
  • LogMessageVM and LogMessageM
  • TimeOutWindowVM and TimeOutWindowM

And those are all my MVVM classes in my application excluding the Views.

Thank you.

Update 3

I still need some claifications.

  1. I still did not understand if I should have collections of Models, collection of ViewModels, or of both.

  2. How can I improve collection synchronization? Especially, how can I do synchronization more modular for situations when I have 4 collections like in the figure in my question? I want especially to know what is the logical difference between synching 2 collections and more than 2, such as 3-4.

I hope these are my last questions in this topic.

Thank you.

6
  • Welcome to Software Engineering! Because your description is a bit complicated and I cannot say I have fully understood it, I will attempt to give you a more generic answer. Some rewording might help, but I do not feel I can give much guidance right now, only some clues. For example, after reading the entire question, I cannot yet "feel" what Clock stands for in your problem domain, or, if it is not a true clock (which is the most probable case), why it was named as such and what it represents. Commented Sep 29, 2019 at 2:04
  • @VectorZita Please let me know if I can offer you more details, or how I can help you help me. Thank you very much. Commented Sep 29, 2019 at 14:34
  • Sure! Let me take a look at your new question revision and I will let you know. Commented Sep 29, 2019 at 14:38
  • 1
    You know, of course, that there's no one single "right" way to do this, right? Commented Sep 29, 2019 at 16:22
  • @VectorZita Please tell me what other parts of the code you need to help me. Related to the big comment in the code snippet in your answer at the moment of this writing, I use data-binding, sometimes two-way data-binding, so I cannot use just computed properties. Thank you. Commented Oct 1, 2019 at 13:45

1 Answer 1

1

Judging from the complexity of your example, which, by the way, I am not fully able to grasp, it seems that you need some general advice on dealing with things the "MVVM way".

First of all, you have ClockGroupM inheriting from BindableBase. If your M suffix really stands for Model, either you are not clear in what a Model is, or you are misunderstanding the MVVM pattern. Your Model is a representation of the problem domain you are dealing with, which is totally ignorant to the fact that View-Models exist.

The MVVM pattern considers Models independent of (pretty much everything, but generally) ViewModels and Views. By inheriting from BindableBase, and then inheriting directly from your model, you pretty much mix your Model and your ViewModel and you are effectively depriving yourself of some of your strongest advantages. The most important problem you are creating is the need to synchronize a wider and deeper object graph.

Your ViewModel objects can and probably should contain your Model objects (but it is not a good idea to inherit from them because it is a large burden of complexity to maintain, so composition is preferable), because its primary role is to act as a "translator" of the Model object with respect to its properties that are relevant to the view. Synchronization is not necessary when you are directly retrieving properties from your Model. So you only have to synchronize the Model objects and request from the ViewModel to update the View. This update will re-retrieve any relevant property from the Model objects and, because the Model objects are synchronized, the View will simply be (as it should be) a visual representation of the current state of the Model.

Generally, the synchronization should generally be mostly a concern of the Model objects. A ViewModel object should only be a "translation" of a Model object that exposes any relevant properties with respect to a specific view-related use case. Different views of the same Model object should be served by different ViewModel objects, all constructed using the same Model object reference.

To directly comment on your case, I suspect that your architecture could benefit from some refactoring. You should not have to synchronize such a convoluted chain of objects, especially not through a static method. I do not know the time constraints of your project(s), so maybe your solution is OK for now.

I really cannot help you further, unless you provide some more code and detail about the synchronization process taking place, as well as a few more details regarding your architecture.

Update 1

It seems that your logic is far more convoluted than what was originally apparent. Some evident problems with your logic, which, to varying degrees, reduce the maintainability of your code, are:

  • Static method to synchronize objects. Among other things, this method takes code away from where it ought to be, i.e. inside the relevant classes. Any structural, functional or "relational" change made to the involved classes necessitates a revisit of this large static method. Needless to say that the more complex you make this method, the more you couple it to yourself as the sole maintainer. Action<U> afterAddition and Action<T> prepareDeletion makes it rather clear that the types T,U matter, their passed order to the method matters, AND they can be null. This shows that only you know what is going on in there and just the argument types are not enough, you need complex parameter names to help someone understand. Remember, when the parameter names of a method gain significance as conveyors of meaning, your methods are starting to get a bit out-of-place.

  • internal... does this serve any essential purpose there? The internal access modifier is one of the textbook code-smells produced by beginners who just like the conveyed meaning. There are rare valid uses for this access modifier, so rare, in fact, that it acts as a double-edged sword. If you must use it, a good chance exists that you are messing your code up. On the other hand, if you don't have to use it, but you do use it, a good chance exists that you are a beginner, still attracted by shiny features that were actually introduced to ease your life at far more advanced stages of development than where you are using them.

  • You are instantiating model parameters inside your ViewModel classes. This does not improve your code. It just mixes up your Model and your ViewModel. If you have to create Model objects inside your ViewModel, your ViewModel becomes involved in the Model processes. Now you have to maintain/synchronize it too. But you were trying to avoid such a thing, right? A ViewModel only exposes properties of a Model, in order to produce a use-case-specific View of the Model.

  • You are creating a ...VMCollection that inherits from BindableBase. "Reactive" collections inherit from ObservableCollection<T>, not from BindableBase, at least if you are trying to achieve a collection that reacts to "Changed" events. Of course, I understand that you already have ObservableCollection objects inside your ...VMCollection classes. However, this is just "involving" the logic even more, as you are giving your classes multiple responsibilities.

  • You have public fields (see public ClockGroupMCollection MyModel;). There should be (almost) absolutely no reason for such a thing. A field can be reassigned at any time and without any potential for interception. This can introduce very subtle bugs (granted, in relatively large codebases) and make debugging sessions a living hell.

  • You are using a LINQ query, an actual LINQ query, using query syntax, rather than method syntax (items.Where(...).Select(...)). Typically, method syntax is easier to get a grasp of and most developers find it easier to use, unless they want to leverage the special syntactic potential of query syntax. You are going to find method syntax used far more often, that is.

  • You are using casts and type checks. When single-dispatch is not enough and you have to resort to casts, you have to have a very good reason, because type checks easily introduce some of the most hard-to-find bugs.

Most of these constructs increase code complexity and make maintenance increasingly difficult as they accumulate. As a result, they are generally preferred only in the absence of clearer and simpler alternatives. Even then, some of these techniques are not to be abused. From the simplified view I can get, based on what you have posted, I think you can benefit by simplifying some parts of your codebase.

Anytime one of these constructs appears to be a good fit, especially when you are starting out and have all the potential to get carried away, you need to step back and reconsider your problem and if you are expressing it properly. If you are needlessly complicating your problem, the solutions will get more and more complex, just as needlessly. For example, in your synchronization method, you request afterAddition and the prepareDeletion actions, which can also be null. This means that you occasionally need to do pre-delete and post-add actions for some reason and because you have spread the process in multiple places, you need to send this action along too, as you are writing generic code and you cannot know what to do there. This is awkward and the Action<T> and Func... constructs may be great but they are not to be abused. You should consider the possibility of performing the post-add and pre-remove actions in the same code location where the removal/addition take place.

My suggestion is to take a step back and reconsider the design a bit. I am going to make some general propositions, which might help you or not.

  1. Build your Model classes, entirely, forgetting about ViewModels for a while. The Model must be able to exist in the full absence of ViewModels, Views, MVVM etc. The first participant in an MVVM pattern is the Model, because it already exists when you decide to visualize it (at least in your mind). As a result, consider restructuring the Model classes of the object graph you have constructed as a model of your problem.

  2. Whenever you need synchronization, consider the why. When you need to synchronize two objects that are totally ignorant to the existence of each other, then and only then should you use events. In any other case, synchronization can be avoided by retrieving whatever is needed "just-in-time", upon request. Consider the following very simplified code example:

public class Clock
{
    public DateTime Time { get; private set; }

    public Clock(DateTime time)
    {
        Time = time;
    }

    public void ChangeTime(DateTime newTime)
    {
        Time = newTime;
    }
}

public class ClockViewModel : BindableBase
{
    private readonly Clock m_clock;

    public ClockViewModel(Clock clock)
    {
        m_clock = clock ?? throw new ArgumentNullException(nameof(clock));
    }

    //If the inner clock reference has changed its time through
    //interactions with the rest of the model, you don't have
    //to update the view model. The view model always retrieves
    //the latest value. Equally, when the model clocks change
    //type, anything relevant in the view-models will be retrieved
    //by the Model clock references. You are free to play with your
    //Model classes and their methods, the ViewModels only expose
    //some properties and might perform additional logic, only
    //related to the use-case/view. In all cases, you can retrieve
    //the most recent property values from the inner model.
    public DateTime
    {
        get => m_clock.Time;
    }
}
  1. What happens when Model classes are out-of-circulation and new ones emerge? Well, what do you do with the Model classes? Right, it's bye-bye! Well, you say goodbye to your ViewModels too! Throw them away (just make sure their events are clear of event listeners). Replace them with new viewmodels, which receive the new model objects in their constructors.

Try to see if these suggestions are helpful to you thus far. I may revise the answer with additional suggestions, but it is important to know if this is helping or simply confusing...

Update 2

First of all, let me ask your specific questions one by one:

  1. If you use binding, the best way is definitely to implement INotifyPropertyChanged. BindableBase may probably be one such boilerplate implementation, so you should be fine with that. Synchronization should not be necessary in most occasions. Generally, if you can do without specialized synchronization, you will be far better.

  2. Changes are propagated to the UI through the ViewModel but the typical way is not through specialized events for each thing you want to update. The VM only "transfers" the value of the property to the UI directly. There already is an event which is specifically for that purpose and it is the PropertyChanged event. The BindableBase class, as is typical for INotifyPropertyChanged implementations, contains a method, the job of which is to say: "Look, Mr. View, the supporting Model has changed, you should be updated". This method is the RaisePropertyChanged(string) method. The string that is passed contains the name of the property that should be updated. Of course, when the model changes without the ViewModel knowing it, you have to rely on events exposed by the model. Take a look at the following simplified example code.

public class Person
    {
        private string m_name;

        public event EventHandler NameChanged;

        public Person(string name)
        {
            //Omit null-check for simplicity
            m_name = name;
        }

        public string Name
        {
            get => m_name;
            set
            {
                m_name = value;
                NameChanged?.Invoke(this, EventArgs.Empty);
            }
        }
    }

    public class PersonViewModel : BindableBase
    {
        private Person _person;

        public string Name => _person.Name;

        public PersonViewModel(Person person)
        {
            _person = person ?? throw new ArgumentNullException(nameof(person));

            _person.NameChanged += OnPersonNameChanged;
        }

        private void OnPersonNameChanged(object sender, EventArgs e)
        {
            //The person name has changed. There could exist a string variable
            //in the viewmodel, which has to be synchronized with the underlying
            //corresponding Person name string variable. However, it is better to
            //skip the private field that would have to by synchronized. The
            //Name property above is just a retrieval method, no backing field is used.
            //However, the View must also be updated. The view "listens" to the
            //PropertyChanged event. If this event is raised, the view will be updated.

            RaisePropertyChanged("Name"); //This causes the View to re-get the Name property.
            //The Name property retrieves the underlying person name property.

            //Or, even better:
            //RaisePropertyChanged(nameof(Name));

            //An alternative would, of course, be to have the event "supply" the new name
            //but this would only be useful if something would have to be synchronized.
            //No field synchronization is necessary here, because all the VM does is to
            //connect specific model information to the View through proper "getter" methods.
        }
    }
  1. Changing the Model through the database is typically not something you do. The database stores everything and changes are initiated from some place outside the database. If I get this right in your case, it's from the User Interface, right? If all changes take place through the user interface, then you change it from your ViewModels, which, of course, have two-way bindings. Therefore, in the setters of your ViewModel properties, which are bound to corresponding element properties on your WPF controls, you should change the underlying model instance properties. The model property setters should probably raise other events to inform other dependent objects that the changes have occurred.

  2. Of course you can have Model objects separately from their corresponding ViewModel objects. The Model objects are independent of ViewModel objects. ViewModel counterparts are only useful for depicting the Model to a User Interface (View). Apart from that, the Model might serve other use-cases as well. If your Model objects are simply collections of properties retrieved from a database, however, ViewModels may appear to be redundant. However, it is a good practice to keep them separated for reasons of maintainability and extensibility. Right now, I see you have inherited from BindableBase from some of your Model objects too. Is this because you want to raise an event any time a property changes? If this is intentional, then you can use that to synchronize relevant activities. You can listen to the PropertyChanged event of your Model objects from other Model objects and, upon every change, synchronize whatever it is that has to change.

Generally, with respect to your ending comments, you will have an easier time propagating changes from your View directly to your Model objects, if your primary purpose is to store them back to the database. Consider the following simplified code example (much like an extension of the previous one):

    public class PersonViewModel : BindableBase
    {
        private Person _person;

        public string Name
        {
            get => _person.Name;
            set
            {
                _person.Name = value;

                //Because it is known that this will cause the NameChanged
                //to be raised, the listener below will make sure that this
                //method is called. If no such event was available, the
                //setter would have to call the method normally.
                //RaisePropertyChanged(nameof(Name));
            }
        }

        public PersonViewModel(Person person)
        {
            _person = person ?? throw new ArgumentNullException(nameof(person));

            _person.NameChanged += OnPersonNameChanged;
        }

        private void OnPersonNameChanged(object sender, EventArgs e)
        {
            RaisePropertyChanged(nameof(Name));
        }
    }

You can see how the UI change propagates through the ViewModel setter (if the Binding is Two-way) directly to the Model object. So, at any time during your application session, you only have to deal with your Model objects. So you can store their changes back to the database, but, more importantly, you never have to worry about synchronizing UI-caused changes, because they propagate directly to the Model. While this scenario may not be able to serve all cases, it is one of the simplest and should not be made more complicated unless you definitely have more complicated requirements.

4
  • I have added more details to the question and a link to the full source code of the 8 classes. I can post any piece of code as necessary. Thank you. Commented Oct 1, 2019 at 13:47
  • No problem! Good, I will take a look and I will try to update my answer accordingly (or post a new one if I think I can make good, self-contained suggestions). Commented Oct 1, 2019 at 16:27
  • Thank you for your continuous help. I have put my, hopefully last, 2 subquestions in the last part of my question. Please, answer them. I hope that with those answers I can do the refactoring well. Thank you. Commented Oct 3, 2019 at 11:54
  • You are most welcome! I will take a look at them as soon as possible! Commented Oct 3, 2019 at 14:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.