This is related to T16895.

I agree that rewriting the basic classes in MitkCore would be a long and cumbersome work, but it is not so difficult, and it can be done systematically. In turn, you could get a much more robust and responsive application, and you could save a lot of time that would be spent with debugging later, otherwise.

This is a kind of job that could be done in small amounts on bug squashing parties. ;-)

T16895 is also the same problem. I described the issue and sent a PR almost three years ago. (It was not even commented on.) This ticket proposes a more general solution, though.

It would be really good to go through the code and fix these bugs systematically, at least in the core, as it causes various crashes, slower application, long debugging hours and the need for all sorts of workarounds to handle the mess.

In general, I like the idea of having the possibility to block/postpone events. The event queue idea must be thought through, though. For example, if an itk::ModifiedEvent is pushed into the queue and there already is another itk::ModifiedEvent somewhere in the queue, the older one should be removed in favor of the latest one. Otherwise it could happen that when unblocking the events, n itk::ModifiedEvents get fired and we didn't win anything. Also the queue should be locked as soon as an itk::DeleteEvent was pushed.

I'm note sure if we can safely make these kinds of assumptions, as it might be important to keep all itk::ModifiedEvents, as there are other events in between that expect up-to-date observers at the time they get fired.

This proposal is not a feature request but it describes a quite a sever bug with a lot of unforeseeable consequences. I try to explain it better.

If the object has atomic state (only one publicly visible data member) then this is not an issue.

If the object has compound state, I would distinguish two cases. If there is no dependency between the members of the state (still talking about publicly visible members), then this reduces simply to a performance issue. I think, this is what you meant. You can change the members independently, the Modified signal may be sent out too many times, unnecessarily, but the observers will always find the object in a consistent state.

However, in many cases there is dependency between the data members. In the previous example, the slice navigation controller modified its stepper before it initialised the geometry of the renderer. This state is inconsistent. There should not be any signal sent out when the object is in an inconsistent state. Not just Modified signal but nothing. Otherwise, there can be someone listening to the signal, checking the object and so on.

If the queue contains every event then we solve the second problem (atomic state transition, no signals while in inconsistent state) but not the first one (performance).

I proposed to keep one event per type. This is good if the events do not have a state, like itk::ModifiedEvent. If the events can have state then maybe last one should be kept for each type.

I just realised that I already created an issue for this early last year:

T17295

The issue has been closed as 'Resolved' without taking any action. The functionality of the class that I used in my example (mitk::DisplayGeometry) has been merged to other classes (e.g. mitk::BaseRenderer), and the class has been removed.

But I am pretty sure that the same bug is still there, just in a different class now.

And T17295 was not about that specific example but I described the same design principle to fix similar bugs consistently throughout MitkCore.

I do not think that the comment on DeleteEvent is valid.

You block the signals on an object that you want to change and unblock the signals when you finished. If the object can be deleted in the middle then something is seriously wrong in your code. You can assume that you own the object that you want to change. In many cases it is the object itself that should block the signals.

Isn't it a general design problem? One could compare this to the strong guarantee of exception safety, i.e., you should not change the actual state of a class in a method until the "throwable" code of the method executed completey. In many cases it is just enough to work with intermediate variables and at the end of the method to safely assign the variables to the class members. That said, anytime an event is fired, a class shouldn't be in an inconsistent state. It never should. I didn't look into the example you mentioned above yet, but isn't it possible to solve by setting the members directly instead of using the public setters and at the end manually call Modified()? If it isn't possible to always have a class in a consistent state at least between method calls, the class / its interface is designed badly.

This is how I would try to solve the actual problem. This doesn't address the performance issue, though, which is a different topic anyways. Blocking/unblocking events should only be a performance tune (and therefore a (nice) feature request), not a solution to badly designed classes.

Yes, it is a general design problem. It is an anti-pattern, I would say. A big NO. :)

In my example it is *not* possible to modify the members directly and at the end call Modified() manually. It might be enough in other, very simple cases. However, there are two problems with this approach.

1. It works when the object wants to block its own signals, but not when it wants to modify another object. See what I called the 'basic solution' above. In my case the slice navigation controller is being reinitialised. It changes its stepper then it changes the geometry of its renderer and it probably changes some of its own members as well. The slice navigation controller should block the events from the stepper, from the renderer and from itself when it starts modifying them and it should unblock these events when it finishes.

2. Having a setter is actually a good idea. I dislike macros in general, but in this case you could enforce this design principle with a setter macro. Just instead of sending the Modified() event from the setter, the setter should pass the Modified() event to a function. This new function would send out the event straight away if the events are not blocked, otherwise it would 'queue' it, and it would be send out when the events are unblocked.

Another thing.

I saw when I was debugging that you guys tried to keep the number of Modified() calls low and sometimes I found it at unexpected places. It was there because a complex initialisation finished there after a long call chain. This is fine, but it is really hard to maintain as you cannot really tell from a static source code always 'where' the execution will finish. For example, if there were several exit points in a function, there was one Modified() call before each. I hope it makes sense.

If you would 'send or queue' these events, it would not matter how many times you do it, the event would be sent out only once and at the right place. So, you could call this MarkModified() or Send(ModifyEvent()) function as many places as you want.