IBaseNode update system seems inconsistent

[DavidLeoni]

IBaseNode updating system seems to have some inconsistency (AA: See also review of PR #6). Maybe it works as intended, for now I put here a quick review of the findings, we will improve it when we find the time.

Desiderata

Here are the desiderata as I understand them

• Changes to IBaseNode are supposed to be done during filtering phase in a controlled way, because when the tree is changed, a call to the nlp preprocessor should be scheduled.
• multiple changes to the tree should lead to just one NLP preprocessor pass
• NLP preprocessor call should be scheduled when there are changes to the node fields (i.e. label). Currently changes to structure don't seem important. TODO: Are there other needs for firing events?

Current situation

IBaseNode has these implementations:

IBaseNode
   |- BaseNode
         |- Node
         |- NLPNode

IBaseNode

IBaseNode has create, add, insert, remove child which is fine. It also has setChildren which might cause problems if not implemented correctly as the user might later change the array

BaseNode

This setter does not trigger any event, looks like a bug. Also user might keep the pointer and change the array.

   @Override
    public void setChildren(List<E> children) {
        this.children = children;
    }

These setters look suspicious to me. The just change fields, not the structure yet I wonder if they should call fireTreeStructureChanged anyway :

   @Override
    public void setId(String newId) {
        id = newId;
    }

    @Override
    public void setName(String newName) {
        name = newName;
    }

    @Override
    public void setUserObject(Object object) {
        userObject = object;
    }

   // David:  this is ok, triggers event
   @Override
    public void setParent(E newParent) {
        removeFromParent();
        parent = newParent;
    }

     // David: this one loops with itself (according to Eclipse) !!!
    @Override
    public void setParent(MutableTreeNode newParent) {
        if (newParent instanceof IBaseNode) {
            setParent(newParent);
        } else {
            throw new IllegalStateException();
        }
    }

Node

TODO: check Node setters and in particular Node.setIsPreprocessed

NLPNode

TODO: What's this for?

TODO: What's the difference between INLPNode.setLabel and IBaseNode.setName ?

NLPNode has also this setter that doesn't call fireTreeStructureChanged:

   public void setLabel(ILabel label) {
        this.label = label;
    }

[autayeu]

Currently changes to structure don't seem important

Structural changes change formulas, so NLP is not needed, but formulas need to be updated anyway through a call to Context Classifier.

// David: this one loops with itself (according to Eclipse) !!!

@OverRide
public void setParent(MutableTreeNode newParent) {

No it's not - signature is different. Notice MutableTreeNode. This call is needed to support trees in GUI. GUI trees are made of MutableTreeNodes and I was trying to use the same data structure to avoid copying data to-from GUI.

NLPNode
TODO: What's this for?

NLPNode is a node with NLP data structures for doing things like annotations - notice it's in s-match-nlp-annotation.

TODO: What's the difference between INLPNode.setLabel and IBaseNode.setName ?

Name is a string. Label is an ILabel instance, which has extra data, like annotations.

NLPNode has also this setter that doesn't call fireTreeStructureChanged:

Notice it's called fireTreeStructureChanged, so it's only for structural changes like adding or removing children. As far as I recall, current usage agrees with this semantic.