Unidata / IDV

The Integrated Data Viewer (IDV) from Unidata is a framework for analyzing and displaying geoscience data.
http://www.unidata.ucar.edu/software/idv/
Other
79 stars 37 forks source link

Make StateManager's processPropertyTable static. #82

Closed jon4than closed 10 years ago

jon4than commented 10 years ago

This resolves some problems described in:

http://mcidas.ssec.wisc.edu/inquiry-v/?inquiry=1483 http://mcidas.ssec.wisc.edu/inquiry-v/?inquiry=1768

This should probably result in #80 being closed…

yuanho commented 10 years ago

It is not clear why the change of api signature can fix this problem.

jon4than commented 10 years ago

80 specifically introduces bugs with bundles either not saving or loading the ViewManager's boolean properties at the right time (due to waiting for getIdv() to return something non-null).

Making processPropertyTable static essentially means that the boolean property initialization can take place without requiring the an instantiation of the StateManager class.

yuanho commented 10 years ago

Does it mean I need to revert the merging of #80? What I don't see is that changing to static not related to the saving data in the bundle.

jon4than commented 10 years ago

Yes, I believe #80 will have to be reverted (it might be easier to just make the changes from this pull request on top of the merged code--thought maybe git/github are smart enough to Just Work?).

You are right in that my change is not related to saving a bundle (I apologize for misspeaking). It only makes a difference when loading. IIRC the problem that we were seeing in Inquiry 1483 was (superficially) caused by methods being defined like so:

    public void setWireframe(boolean value) {
        setBp(PREF_WIREFRAME, value);
    }

    protected void setBp(String propertyId, boolean value) {
        getBooleanProperty(propertyId).setValue(value);
    }

That getBooleanProperty call eventually gets us to the method changed as part of #80. Prior to #80, if initBooleanProperties was called before the idv field was set, the getStateManager() call within initBooleanProperties would return null, resulting in an NPE when stateManager.processPropertyTable(...) executes.

I happened to notice that processPropertyTable could be made static and function exactly the same as before, without having to worry about whatever getStateManager() might return.