Week 34 of 2009

This week I started with a nice Costa coffee (I like Starbucks but their cafe in Colchester has unfriendly staff and poor seating). I should have been nailing down the outstanding issues holding back Build 014 of OOA Tool. Instead, I had a think about some of the annoying design decisions I made when I started creating the model elements that realize the OOA of OOA within OOA Tool.

One of the most annoying (for me) is that I associate a dynamic parent with each model element. On reflection, a static unchanging parent is much safer and entirely acceptable for OOA Tool. You might not think it makes much difference. However, there is an enormous amount of listener code within these model elements allowing derived fields to push their changes out to GUI listeners which update the GUI as and when changes occur. There is also lots of unnecessary parent status checking. I also take advantage of the fact that the parent of a child is cleared when it is removed from the parent. Of course, removing the parent can cause problems with the deleted model elements which may still be in use! Thankfully, programmer deletion is logical rather than physically in Java so I don't need to worry about memory being reclaimed while still in use.

I also associate a changed status with each model element which I recursively clear after a project is loaded or saved. However, I hadn't got around to setting the changed status when non-derived fields are changed. An annoying side-effect of this was that I couldn't check whether any projects had been changed when the application is exited, i.e. a confirmation dialog is always popped up - very annoying.

A cut down outline of the ModelElement class is shown below:

public abstract class ModelElement {
    private ModelElement m_parent;
    private boolean m_changed;

    protected ModelElement() { }

    public ModelElement getParent() { }
    public void setParent(ModelElement parent) { }
    public abstract ModelElement[] getChildren();

    public boolean isChanged() { }
    public void setChanged(boolean changed) { }
    public boolean isChangedAnywhere() { }
    public void setChangedRecursively(boolean changed) { }

    protected void firePropertyChange(String propertyName,
            ModelElement oldValue, ModelElement newValue,
            boolean child) {
        if (oldValue != newValue) {
            m_propertyChangeSupport.firePropertyChangeEvent(
                propertyName, oldValue, newValue);
            if (child) {
                if (oldValue != null) {
                    oldValue.setParent(null);
                }
                if (newValue != null) {
                    newValue.setParent(this);
                }
            }
        }
    }
}

A cut down outline of the new improved ModelElement class is shown below:

public abstract class ModelElement<P extends ModelElement> {
    private P m_parent;
    private boolean m_deleted;
    private boolean m_changed;
    private boolean m_changedAnywhere;

    protected ModelElement(P parent) { }

    public P getParent() { }
    public abstract ModelElement[] getChildren();

    public boolean isDeleted() { }
    public void setDeleted(boolean deleted) { }
    public void setDeletedRecursively(boolean deleted) { }

    public final boolean isChanged() {
        return m_changed;
    }
    public void setChanged(boolean changed) {
        boolean oldChanged = m_changed;
        if (changed != oldChanged) {
            m_changed = changed;
            m_propertyChangeSupport.firePropertyChange(
                CHANGED_PROPERTY, oldChanged, changed);
            ModelElement modelElement = this;
            while (modelElement != null) {
                modelElement.setChangedAnywhere();
                modelElement = modelElement.getParent();
            }
        }
    }
    public final boolean isChangedAnywhere() {
        return m_changedAnywhere;
    }
    public void setChangedAnywhere() {
        boolean changedAnywhere = m_changed;
        if (!changedAnywhere) {
            for (ModelElement child : getChildren()) {
                if (m_child.m_changedAnywhere) {
                    changedAnywhere = true;
                    break;
                }
            }
        }
        boolean oldChangedAnywhere = m_changedAnywhere;
        m_changedAnywhere = changedAnywhere;
        m_propertyChangeSupport.firePropertyChange(
            CHANGED_ANYWHERE_PROPERTY,
            oldChangedAnywhere, changedAnywhere);
    }
    public void setChangedRecursively(boolean changed) {
        setChanged(changed);
        for (ModelElement child : getChildren()) {
            child.setChangedRecursively(changed);
        }
    }

    protected void firePropertyChange(String propertyName,
            ModelElement oldValue, ModelElement newValue,
            boolean child, boolean derived) {
        if (oldValue != newValue) {
            m_propertyChangeSupport.firePropertyChangeEvent(
                propertyName, oldValue, newValue);
            if (child) {
                if (oldValue != null && !m_deleted) {
                    oldValue.setDeletedRecursively(true);
                }
                if (newValue != null && !m_deleted) {
                    newValue.setDeletedRecursively(false);
                }
            }
            if (!derived && !m_deleted) {
                setChanged(true);
            }
        }
    }
}

This requires parents to be statically declared using Java generics and requires the parent to be passed to the constructor. The only root element without a parent being a Project in OOA Tool. However, it is still useful to know when a model element is removed so a new deleted status was added. The setting and clearing of this status is done when property changes are fired. I have also been able to clear up some minor memory leaks by making use of the deleted status to add or remove nested listeners in line with the deleted status.

I also added another flag to firePropertyChange() indicating whether a derived change has occurred. This allows us to set changed statuses automatically as a consequence. Since we have a fixed parent now we can also fairly easily determine a changed anywhere status for each model element where anywhere indicates at or below a given model element. The old design required a recursive check to be performed on request to determine whether changed anywhere. OOA Tool now adds a changed (or changed anywhere) overlay icon to tree browser icons when changed or changed anywhere statuses are updated. It can also be determined if any projects have changed when the user exits the application, i.e. no redundant dialog. However, there is a limitation here in that if a change is made and another change follows it which undoes the previous change, the model element will still be flagged as changed. This doesn't occur in any of the editor dialogs since they record the original values and always check the current values against the original values when determining whether a change has occurred.

These changes (80% done now) were for the most part straight forward except for the fact that there are 336 model element classes (744 classes in total)! However, there were a few places where I unnecessarily took advantage of dynamic parents and this code needed to be rewritten.

OK, this was a serious side-track from my current priorities. However, I am very happy with the changed and changed anywhere indicators in the tree browser. I am also much happier that the parent is always set now and its type can be statically determined reducing the need for so many casts.