3

I am working on a WPF application in which I have to group a viewmodel(GroupedViewModel) based on collection of other viewmodels (ChildModel).

Collection of viewmodels and Property changes:

ObservableCollection<ChildModel> chlidModelCollection;
//initialize child model 
childModel.PropertyChanged += OnChildModelPropertyChanged

public void OnChildModelPropertyChanged(object sender, 
PropertyChangedEventArgs args)
{
    //try grouping GroupViewModel
    //do some tasks which takes little over time
}

As Child Viewmodel has 10 properties any change to property calls the OnChildModelPropertyChanged 10 times. For collection of 10 child models in the collection, OnChildModelPropertyChanged is called 10*10 = 100 times. This is impacting performance.

Is there a way to get aggregated or final changeset just once for all the property changes in child viewmodel collection. Am I missing something?

Sharad Shahi
  • 119
  • 1
  • 9
  • 1
    What are you doing in OnChildModelPropertyChanged? Do you need to execute this method whenever *any* property changes? – mm8 Sep 28 '17 at 15:29
  • Why is it implemented in such a way that a change to _any_ property triggers the `OnChildmodelPropertyChanged` 10 times? – Jason Boyd Sep 28 '17 at 15:30
  • @mm8 OnChildModelPropertyChanged, if some properties are similar across the child view model collection, it is grouped and the properties are set in GroupViewModel. – Sharad Shahi Sep 28 '17 at 15:33
  • @JasonBoyd If new properties are added in the child viewmodel the logic will work – Sharad Shahi Sep 28 '17 at 15:35
  • There is `BeginUpdate/EndUpdate` pattern used to solve exactly same performance issue earlier in winforms. In wpf updates are deferred (occurs via queue) and are optimized (multiple changes will cause just one layout/render pass). But we don't know what are you doing. You may have to implement either winforms or wpf way to viewmodels to fix your performance issues. – Sinatr Sep 28 '17 at 15:46
  • What you want is called "debouncing." It's the process of throttling updates when they come at you too quickly. Nothing exists in the framework that does this for you (at the current time, that I've ever seen). You have to write it yourself. You can find lots of examples here like https://stackoverflow.com/questions/28472205/c-sharp-event-debounce –  Sep 29 '17 at 16:47

2 Answers2

8

Don't do anything expensive in response to the events

If they are just changing the values in the view and viewmodel, even if these are assigning values to model objects, you are probably implementing a 'save on change' which does something expensive such as I/O to a database or networking to a service.

Alternately, you could be changing other properties in response to user events, e.g., enabled or visible state of other applicable items.

Unfortunately, if you just take these things out, your app will break. Read on for alternatives...

Solution with a UI change

If you can change the UI, add a Save button to defer IO/networking events until the user is ready to do this. This is a rather unsavory option.

Same solution - but without the UI change

You could also just set a 'dirty' flag, and autosave every 10 sec via a timer, and also autosave on close of the form. That would be less intrusive.

Only save when something really changes

Autobinding will, over course, autobind. However if your object structure allows detecting changes and comparing the original value with the changed value, without a million lines of monkey-code, consider deferring expensive operations unless something really changed. It's impossible, of course, for a user to change more than one field at a time. You're not stupid though, so you know that. My guess is the loading or adding a section of the UI is firing the events, so...

Skip expensive operations during automated changes

If the events are firing during the population of the UI from the data store, or a user action causes automated setting of many fields, make sure you have code in place to identify those events. Then set a member variable that's visible to all the UI that you can check before doing expensive things like IO or networking. These are 'app initiated' changes, even if they are in response to a user action on the screen.

When the app initiated events are complete, then call the expensive operation once from the code that initial ran in response to the single user-initiated event.

Make all property mods to the entire UI from one method invocation only

(tl;dr for this section - architectural divergences with MVVM make this less practical without losing its advantages, but you might get some good takeaways)

This can be a method that calls other methods, of course, as best practices and repeated UI parts dictate. This is a philosophical thing because responding to updates just for that section of the screen can occur in different orders, if different sections of the screen have to start interacting with each other, as per business requirements, this is like different objects effecting each other and the side effects explode.

You would not allow that with objects - you would make methods and members private. Unfortunately on a UI you don't have a choice.

The upshot is that as different events can fire in different orders, it increases a key indicator of poor reliability/maintainability, and debuggability called 'cyclometric complexity' - since there are so many different paths thru the code.

Usually this is caused by IF statements in methods. Unfortunately, with events, it's the same problem, but hidden. Also, there's no way to code around it with best practices. It's just a problem with the concept of event-driven UI programming (and to some degree, over-use of events in SOA fire & forget architectures). It applies to event driven programming in everything from VB6, to WPF to JavaScript with Jquery or Angular handling the events.

The solution is to (somehow) let all the events fire, and then just once call a method which does all the UI setup. This requires first doing logic which determines which controls need to be there, determining what events fired on them, and then doing additional setup on them, all the while ensuring that during this step, events are not firing (which would run everything over again).

The problem with doing this in WPF is that binding occurs in response to events, not at your control. But note this:

binding is always good!

responding directly to data changed events gets bad for complicated UIs (like yours) Or, for that matter, any complicated system that fires and responds to asynchronous (or similated async) commands, events or messages without very careful 'walls' to reduce the scope of the events.

I'm not enough of a WPF dev to know how to have your view update downstream models without responding to events.

What is needed is a lifecycle that can be initiated by events, but only cares in a small subset of cases, and runs all the logic for the screen that would run if any other field changed. The reason for this is that usually, the field that changes doesn't matter. Think about that - when you load the UI from the data source, does it matter how they entered it? You can share any loading setup code with code to change the screen state in response to data entries. The only events you have to check for are these:

button/link press that's the point - of course you need to know what changed

fields that change each other for example let's say you let a user withdraw from their $1000 CD. if you have $ amount and % amount and want entry of $500 to refresh the % to 50, and entry of 25% to refresh the $ to $250, you have to know which field they change to modify the other one. As you can guess - obscure.

I've worked with a proprietary framework that did autobinding, allowed important data changes to fire off a lifecycle that did the same UI setup and UI reading/saving, and handled other simple things (like graying controls and showing a validation balloon) via an SDK. I long for it every time I am forced to implement anything non-trivial using event driven UI programming.

MVC Websites approach this with their binding mechanisms but the natural 'post/response' cycle of page-based websites can reduce the prevalence of coding in response to events.

This is why WebForms is so difficult - it artificially imposes the troubles of event driven UI onto the top of a naturally simpler system.

Of course with SPA web apps that go to a service, just swap C#/WPF-MVVM with JS/Angular MVVM. MVVM is conceptually cool, the layers are good to have, binding is cool, it's the events that can get hairy with a big UI. And big UIs are needed for tough business cases. You will not see office workers doing their jobs on phones. Some will argue - but they're not the ones paying salaries and responding quickly to customer needs;-)

This isn't meant to be a rant on Event-driven programming. It has its place, it just collapses under the weight of bigger real business UI cases, and needs a lot of propping up. Which we devs are quite good at doing!

In 40 years, people will look at event driven programming they way we look at god-objects and gotos. But if we're not both retired in beach houses by then, I'll eat my mouse! (let me cook it first)

Community
  • 1
  • 1
FastAl
  • 6,194
  • 2
  • 36
  • 60
1

You should look into Reactive Extensions (Rx) if you want to be able to "bundle" all events into just one that you handle any x seconds:

C# Event Handler trigger multiple times, however need to handle it once in 10 secs

If it only makes sense to execute the OnChildModelPropertyChanged when a specific property is changed, you could return from the event handler immediately when it is invoked for any other property:

public void OnChildModelPropertyChanged(object sender, PropertyChangedEventArgs args)
{
    if (args.PropertyName != "ThePropertyName")
        return;

    //try grouping GroupViewModel
    //do some tasks which takes little over time
}

The event handler will indeed be invoked just as many times as before, but the long-running code inside it won't run for all property changes.

You might also consider scheduling/off-loading the work to be executed to the thread pool:

public void OnChildModelPropertyChanged(object sender, PropertyChangedEventArgs args)
{
    Task.Run(() => { /* do something */ });
}
mm8
  • 163,881
  • 10
  • 57
  • 88
  • Thanks!. I have both in place. As the list grows(30 or more) the property changed notifications are too many. All the 10 fields are valid fields for property changed. I am going through Rx. – Sharad Shahi Sep 28 '17 at 15:42