Discussion: Propagation of Spoon model changes

[pvojtechovsky]

Actually the change in spoon model fires the event on FineModelChangeListener, which is registered in Spoon environment.

Problems:

• the change evens are fired even during creation of new elements at the time when they are not connected by a parent to the model
• we cannot improve performance of some opertations by caching data in elements, because these elements are not informed about changes of their children
• .. ?

Idea:
May be we might propagate event about change using the CtElement#parent relation.

while(e != null) {
  if(e.getFineModelChangeListener() != null) {
     e.getFineModelChangeListener().fireEvent(...);
  }
  if(e.isParentInitialized() == false) {
    return; //or send event to Environment?
  }
  e = e.getParent();
}

WDYT?

[monperrus]

interesting.

what would be the fired event ? something like CHILD_CHANGE?

another point. what about moving the spoon.experimental.modelobs in spoon.modelobs? (no more experimental before too many clients use it)

[pvojtechovsky]

what would be the fired event ? something like CHILD_CHANGE?

The nicest would be to propagate all the FineModelChangeListener events through parents only (no direct call of Environment's listener) ... and as last - for backward compatibility - the Environment would receiver the event same like now.

But there are actually many events in FineModelChangeListener` so we should wrap them first into an event object and then we can send this generic event object through the chain of parents. Otherwise the event firing code would have to be copied for each event ... ugly.

another point. what about moving the spoon.experimental.modelobs in spoon.modelobs? (no more experimental before too many clients use it)

Let's wait with #1941 and remove of `experimental' until we finish this discussion and may be refactor firing of these events first ... before client's starts to use it...

[monperrus]

Propagating events to to the parents is a good idea, esp. if the runtime overhead is small.

But I don't see what you're waiting for for #1941.

What's the problem to settle here?

[pvojtechovsky]

what you're waiting for for #1941.

#1941 is about catching of these events ... and if we change the approach how to fire and catch events, then this PR has to be refactored again. It is no problem to refactor ... as long as there are no clients ... and there was a suggestion to move it from experimental package to normal package ...
... all that together is the reason.

@monperrus , @surli just decide the order in which you would like to do next steps.

The simplest baby steps are probably:

1. to merge review: feat: ChangeCollector listens on changes and remembers them #1941 (ChangeCollector) into experimental package
2. to finish review: refactor: mutable collections of Spoon model handles parent and fire change events #1917 and to refactor whole spoon model to use ModelList and ModelSet everywhere so tests in WIP: test: spoon model collections contract #1922 passes.
3. then after everywhere is ModelList and ModelSet it will be easy to change event firing algorithm ... because it will be only on two places (ModelList and ModelSet) and no more spread over whole Spoon model.
4. after event firing algorithm fires one generic event through one method (and not many methods of FineModelChangeListener, then we can propagate events to the parents.
5. we can refactor ChangeCollector
6. we can move it from experimental to standard

... quite a lot of work ... but Spoon will be nicer and more powerful again ;-)

WDYT?

[monperrus]

really nice baby PR plan! I agree :-)

[pvojtechovsky]

We just need some contributors who will implement that ... :-)

[monperrus]

all Github is listening to this conversation :-)  so somebody will pop up!

[monperrus]

[Issue Cleaning Marathon] No more activity here, closing the issue, and of course we can continue the discussion when something new happens.