Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[eclipse-dev] Warning: avoid testing preference names using ==

We have recently seen several instances of a programming error whereby 
implementors of IPropertyChangeListener use == to check the value of 
PropertyChangeEvent.getProperty(), rather than using equals().

This is incorrect because there is no guarantee that the property name in 
the event is a constant.
For example, the Workbench / Preferences / Import... action reads the 
property names and values from a file.  It sends out notifications to any 
activated plugins, but the string read from the file for the property name 
will not be == to the corresponding string constant.
(We considered interning the strings in the preferences import, but this 
is not the right answer and would merely hide the problem until some other 
use case reads property names from a file, not to mention encouraging bad 
programming practices.)

Please ensure that all senders of PropertyChangeEvent.getProperty() use 
equals rather than ==.
Although the class ensures that the property name is not null, a safe 
idiom to use is the following:
if (MY_PROPERTY.equals(event.getProperty())) {
  // update accordingly
}

The preferences facility has recently been pushed down to core runtime 
from JFace, but the JFace APIs still exist and are still commonly used.
So you will need to check references to both:
org.eclipse.jface.util.PropertyChangeEvent.getProperty(), and
org.eclipse.core.runtime.Preferences$PropertyChangeEvent.getProperty()

If this is not fixed, preferences will appear to work normally when being 
adjusted manually in the preference pages.
However, when preferences are exported and reimported, any active plugins 
with listeners using == will not update properly.

We highly recommend testing all your preferences with the new 
import/export facility.

There are existing PRs for known occurrences in the SDK.
See http://dev.eclipse.org/bugs/show_bug.cgi?id=20471

I have filed http://dev.eclipse.org/bugs/show_bug.cgi?id=20758
to clarify the spec for PropertyChangeEvent.getProperty().

Nick


Back to the top