#307 FilterPanel: Listener is triggered multiple times

[Philipp Mewes]

The actionPerformed()-method of PropertyDefinitionChanged is called multiple times. The hash codes of the source and the listener indicates that multiple different ComboBoxes are active, triggering their own listeners.
Each time showFilter() is called, all components are removed from the panel, recreated and readded. showFilter() is also called from the listener's actionPerformed()-methods. This may cause the bug.

[Philipp Mewes]

Seems to be an easy fix. I wondered why the property definition comboxbox was recreated each time showFilter() was called, which was especially everytime, the value of this combobox changes. Moved the creation and configuration of the combobox to the constructor. This does not ony saves some resources, it also fixes the bug, since only a single instance of the combobox with a single listener-instance is presented. See here: [4ee2b0].

The procedure is similar to the recreaton of the other GUI elements at the panel. Even though this seems not to cause any bugs, it could be implemented more efficiently by creating each element only once. Since the code in this class is already touched by this bug, should this be changed too?

[Philipp Mewes]

Fixed two small bugs: [a030b5] and [82bc27].

[Philipp Mewes]

Another fix: [d507ae].

[Till Schäfer]

As far as i understood from a first glance, showFilter is called on initialization and later whenever PropertyDefinitionChanged, NumComparisonChanged or StringComparisonChanged is called, to update the content of the drop down. The problem here is, that showFilter just re-adds the components and does not clean up the listeners on the old components, which causes the GC to not clean them up und causes all the hidden elements to be updated as well. Am I right?
If thats the case, the folowing things should be considered as well:
(1) The problem does not only apply to showPropertyDefinition, but also to showNumFilter, showStringFilter, and showDeleteButton, showNumValue, and showStringValue. I think a good solution here would be to split the functionality in "create" and "update" that is now bundled in #showFilter. Thus, just create the static content in the constructor (as you have done for showPropertyDefiniton/propDefCombo. And just update the content of the panel and the content of the boxes by showFilter which should be renamed to updateFilterPanel then.

(2) An organisational thing: is this easy to seperate this changes from FR 10? In this case, please rebase on master, and merge the changes back into FR 10 as soon this bug is fixed. This allows to merge this bug prior to FR 10.

[Philipp Mewes]

Yes. I think the reason for this is the propertyDefinitionsModel that holds the last reference to the ComboBox. If propertyDefinitionsModel.setSelectedItem() is called, the listeners of all associated ComboBoxes are triggered. In showNumFilter() and showStringFilter() the problem does not occur because a new model is used there. However this methods still cause overhead, since the combo boxes are recreated.

[Philipp Mewes]

(1) Moved the other comboboxes to the constructor ([19c7e9]), which caused a bug which is fixed here: [a19b49]. Also moved stringValue and numValue ([5321a9]).

[Philipp Mewes]

(2) I think rebasing is not possible here. [5321a9] on bug-307 changes the update of the SpinnerNumberModel of the numValue-spinner (see ll 350 - 358). This update is necessary since eature-10 introduces limits for this spinner. If bug-307 is rebased, this change will disappear and must be re-applied on feature-10 after the changes from bug-307 are merged.

[Philipp Mewes]

The latest fix ([370201]) removed this modification. Maybe a rebase is still possible. I will check this.

[Philipp Mewes]

[a030b5] still refers to changes from feature-10. Rebasing bug-307 onto master therefore would lead to an inconsistent history.

[Till Schäfer]

looks good. please merge this back to FR 10 on your own. As soon FR 10 is finished I will close this bug.

[Philipp Mewes]

Merged into feature-10 in [844e93]