fix: The InstanceManager does not provide constants

If JMRI is relying on the InstanceManager to hold some value, we should not be retaining static final references to those objects.
JMRI · Aug 9, 2019 · 32e6c10 · 32e6c10
1 parent 1f5646c
commit 32e6c10
Show file tree

Hide file tree

Showing 13 changed files with 29 additions and 58 deletions.
diff --git a/java/src/jmri/jmrit/ctc/CTCMain.java b/java/src/jmri/jmrit/ctc/CTCMain.java
@@ -5,17 +5,12 @@
 
 package jmri.jmrit.ctc;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.beans.PropertyChangeEvent;
 import java.beans.PropertyChangeListener;
-import java.time.Duration;
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedList;
-import jmri.InstanceManager;
 import jmri.Sensor;
-import jmri.SensorManager;
 import jmri.Turnout;
 import jmri.jmrit.ctc.ctcserialdata.CTCSerialData;
 import jmri.jmrit.ctc.ctcserialdata.CodeButtonHandlerData;

diff --git a/java/src/jmri/jmrit/ctc/CallOn.java b/java/src/jmri/jmrit/ctc/CallOn.java
@@ -121,11 +121,11 @@ public CallOn(LockedRoutesManager lockedRoutesManager, String userIdentifier, St
                         if (ProjectsCommonSubs.isNullOrEmptyString(externalBlockName)) {
                             throw new CTCException("CallOn", userIdentifier, "groupingString", groupingString + " " + Bundle.getMessage("CallOnSignalMastBlockError")); // NOI18N
                         }
-                        Block externalBlock = blockManager.getBlock(externalBlockName);
+                        Block externalBlock = InstanceManager.getDefault(BlockManager.class).getBlock(externalBlockName);
                         if (externalBlock == null) {
                             throw new CTCException("CallOn", userIdentifier, "groupingString", groupingString + " " + Bundle.getMessage("CallOnSignalMastBlockError2"));    // NOI18N
                         }
-                        NamedBeanHandle<Block> namedBeanHandleExternalBlock = NAMED_BEAN_HANDLE_MANAGER.getNamedBeanHandle(externalBlockName, externalBlock);
+                        NamedBeanHandle<Block> namedBeanHandleExternalBlock = InstanceManager.getDefault(NamedBeanHandleManager.class).getNamedBeanHandle(externalBlockName, externalBlock);
                         _mGroupingDataArrayList.add(new GroupingData(signal, trafficDirection, 0, null, namedBeanHandleExternalBlock, route));
                     }
                 } catch (CTCException e) { e.logError(); return; }
@@ -236,6 +236,4 @@ private String convertFromForeignLanguageColor(String foreignLanguageColor) {
         return "Red";   // NOI18N    Should NEVER happen, but if programmers screw up, default to some "sane" value.
     }
 
-    private static final NamedBeanHandleManager NAMED_BEAN_HANDLE_MANAGER = InstanceManager.getDefault(NamedBeanHandleManager.class);
-    private static final BlockManager blockManager = InstanceManager.getDefault(BlockManager.class);
 }
diff --git a/java/src/jmri/jmrit/ctc/NBHSensor.java b/java/src/jmri/jmrit/ctc/NBHSensor.java
@@ -78,8 +78,6 @@ public class NBHSensor {
     public static final String DEFAULT_STRING_RV = "UNKNOWN";  // NOI18N  For any function that returns String.
 //  Functions that don't return any of the above have specific implementations.  Ex: PropertyChangeListener[] or ArrayList<>
 
-    private static final NamedBeanHandleManager NAMED_BEAN_HANDLE_MANAGER = InstanceManager.getDefault(NamedBeanHandleManager.class);
-
 //  The "thing" we're protecting:
     private final NamedBeanHandle<Sensor> _mNamedBeanHandleSensor;
     public Sensor getBean() {
@@ -95,7 +93,7 @@ public boolean matchSensor(Sensor sensor) {
     public NBHSensor(String module, String userIdentifier, String parameter, String sensor, boolean optional) {
         Sensor tempSensor = optional ? getSafeOptionalJMRISensor(module, userIdentifier, parameter, sensor) : getSafeExistingJMRISensor(module, userIdentifier, parameter, sensor);
         if (tempSensor != null) {
-            _mNamedBeanHandleSensor = NAMED_BEAN_HANDLE_MANAGER.getNamedBeanHandle(sensor, tempSensor);
+            _mNamedBeanHandleSensor = InstanceManager.getDefault(NamedBeanHandleManager.class).getNamedBeanHandle(sensor, tempSensor);
         } else {
             _mNamedBeanHandleSensor = null;
         }

diff --git a/java/src/jmri/jmrit/ctc/NBHSignalHead.java b/java/src/jmri/jmrit/ctc/NBHSignalHead.java
@@ -30,8 +30,6 @@ public class NBHSignalHead extends NBHAbstractSignalCommon {
     public static final float DEFAULT_FLOAT_RV = (float)0.0;   // For any function that returns float.
     public static final String DEFAULT_STRING_RV = "UNKNOWN";  // NOI18N  For any function that returns String.
 
-    private static final NamedBeanHandleManager NAMED_BEAN_HANDLE_MANAGER = InstanceManager.getDefault(NamedBeanHandleManager.class);
-
 //  The "thing" we're protecting:
     private final NamedBeanHandle<SignalHead> _mNamedBeanHandleSignalHead;
 
@@ -40,7 +38,7 @@ protected NBHSignalHead(String signal) {
             // Cannot use a constant Instance manager reference due to the dynamic nature of tests.
             SignalHead signalHead = InstanceManager.getDefault(jmri.SignalHeadManager.class).getSignalHead(signal);
             if (signalHead != null) {
-                _mNamedBeanHandleSignalHead = NAMED_BEAN_HANDLE_MANAGER.getNamedBeanHandle(signal, signalHead);
+                _mNamedBeanHandleSignalHead = InstanceManager.getDefault(NamedBeanHandleManager.class).getNamedBeanHandle(signal, signalHead);
                 return;
             }
         }

diff --git a/java/src/jmri/jmrit/ctc/NBHSignalMast.java b/java/src/jmri/jmrit/ctc/NBHSignalMast.java
@@ -33,8 +33,6 @@ public class NBHSignalMast extends NBHAbstractSignalCommon {
     public static final float DEFAULT_FLOAT_RV = (float)0.0;   // For any function that returns float.
     public static final String DEFAULT_STRING_RV = "UNKNOWN";  // NOI18N  For any function that returns String.
 
-    private static final NamedBeanHandleManager NAMED_BEAN_HANDLE_MANAGER = InstanceManager.getDefault(NamedBeanHandleManager.class);
-
 //  The "thing" we're protecting:
     private final NamedBeanHandle<SignalMast> _mNamedBeanHandleSignalMast;
 
@@ -46,7 +44,7 @@ protected NBHSignalMast(String signal) {
             // Cannot use a constant Instance manager reference due to the dynamic nature of tests.
             SignalMast signalMast = InstanceManager.getDefault(jmri.SignalMastManager.class).getSignalMast(signal);
             if (signalMast != null) {
-                _mNamedBeanHandleSignalMast = NAMED_BEAN_HANDLE_MANAGER.getNamedBeanHandle(signal, signalMast);
+                _mNamedBeanHandleSignalMast = InstanceManager.getDefault(NamedBeanHandleManager.class).getNamedBeanHandle(signal, signalMast);
                 _mDangerAppearance = getAppearanceMap().getSpecificAppearance(SignalAppearanceMap.DANGER);
                 return;
             }

diff --git a/java/src/jmri/jmrit/ctc/NBHTurnout.java b/java/src/jmri/jmrit/ctc/NBHTurnout.java
@@ -39,16 +39,14 @@ public class NBHTurnout {
     public static final float DEFAULT_FLOAT_RV = (float)0.0;   // For any function that returns float.
     public static final String DEFAULT_STRING_RV = "UNKNOWN";  // NOI18N  For any function that returns String.
 
-    private static final NamedBeanHandleManager NAMED_BEAN_HANDLE_MANAGER = InstanceManager.getDefault(NamedBeanHandleManager.class);
-
 //  The "thing" we're protecting:
     private final NamedBeanHandle<Turnout> _mNamedBeanHandleTurnout;
     private final boolean _mFeedbackDifferent;
 
     public NBHTurnout(String module, String userIdentifier, String parameter, String turnout, boolean FeedbackDifferent) {
         Turnout tempTurnout = getSafeExistingJMRITurnout(module, userIdentifier, parameter, turnout);
         if (tempTurnout != null) {
-            _mNamedBeanHandleTurnout = NAMED_BEAN_HANDLE_MANAGER.getNamedBeanHandle(turnout, tempTurnout);
+            _mNamedBeanHandleTurnout = InstanceManager.getDefault(NamedBeanHandleManager.class).getNamedBeanHandle(turnout, tempTurnout);
         } else {
             _mNamedBeanHandleTurnout = null;
         }

diff --git a/java/src/jmri/jmrit/ctc/SignalDirectionIndicators.java b/java/src/jmri/jmrit/ctc/SignalDirectionIndicators.java
@@ -18,8 +18,8 @@
 import jmri.jmrit.ctc.ctcserialdata.ProjectsCommonSubs;
 
 public final class SignalDirectionIndicators implements SignalDirectionIndicatorsInterface {
-    static HashSet<NBHAbstractSignalCommon> _mSignalsUsed = new HashSet<NBHAbstractSignalCommon>();
-    public static void resetSignalsUsed() { _mSignalsUsed = new HashSet<NBHAbstractSignalCommon>(); }
+    static final HashSet<NBHAbstractSignalCommon> _mSignalsUsed = new HashSet<>();
+    public static void resetSignalsUsed() { _mSignalsUsed.clear(); }
     private NBHSensor _mLeftSensor;
     private NBHSensor _mNormalSensor;
     private NBHSensor _mRightSensor;

diff --git a/java/src/jmri/jmrit/ctc/SignalDirectionLever.java b/java/src/jmri/jmrit/ctc/SignalDirectionLever.java
@@ -3,10 +3,6 @@
  */
 package jmri.jmrit.ctc;
 
-import jmri.jmrit.ctc.CTCMain;
-import jmri.jmrit.ctc.CTCException;
-import jmri.jmrit.ctc.CTCConstants;
-import jmri.JmriException;
 import jmri.Sensor;
 
 public class SignalDirectionLever {

diff --git a/java/src/jmri/jmrit/ctc/TrafficLocking.java b/java/src/jmri/jmrit/ctc/TrafficLocking.java
@@ -4,7 +4,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import jmri.Sensor;
-import jmri.jmrit.ctc.ctcserialdata.CodeButtonHandlerData;
 import jmri.jmrit.ctc.ctcserialdata.ProjectsCommonSubs;
 import jmri.jmrit.ctc.ctcserialdata.TrafficLockingEntry;
 import org.slf4j.Logger;

diff --git a/java/src/jmri/jmrit/ctc/ctcserialdata/CTCSerialData.java b/java/src/jmri/jmrit/ctc/ctcserialdata/CTCSerialData.java
@@ -13,6 +13,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import jmri.jmrit.ctc.CTCFiles;
+import jmri.util.FileUtil;
 
 /**
  *

diff --git a/java/src/jmri/jmrit/ctc/editor/CtcEditorAction.java b/java/src/jmri/jmrit/ctc/editor/CtcEditorAction.java
@@ -1,8 +1,9 @@
 package jmri.jmrit.ctc.editor;
 
 import java.awt.event.ActionEvent;
+import jmri.InstanceManager;
+import jmri.jmrit.ctc.editor.gui.FrmMainForm;
 import jmri.util.swing.JmriAbstractAction;
-import jmri.util.swing.WindowInterface;
 
 /**
  * Swing action to create and register a CtcEditor.
@@ -21,12 +22,9 @@ public CtcEditorAction() {
 
     @Override
     public void actionPerformed(ActionEvent e) {
-        if (jmri.InstanceManager.getNullableDefault(jmri.jmrit.ctc.editor.gui.FrmMainForm.class) != null) {
-            // Prevent duplicate copies
-            return;
-        }
-        jmri.jmrit.ctc.editor.gui.FrmMainForm f = new jmri.jmrit.ctc.editor.gui.FrmMainForm();
-        f.setVisible(true);
+        InstanceManager.getOptionalDefault(FrmMainForm.class).orElseGet(() -> {
+            return new FrmMainForm();
+        }).setVisible(true);
     }
 
     // never invoked, because we overrode actionPerformed above

diff --git a/java/src/jmri/jmrit/ctc/editor/code/CheckJMRIObject.java b/java/src/jmri/jmrit/ctc/editor/code/CheckJMRIObject.java
@@ -5,11 +5,8 @@
 import javax.swing.table.DefaultTableModel;
 import jmri.BlockManager;
 import jmri.InstanceManager;
-import jmri.SensorManager;
 import jmri.SignalHeadManager;
 import jmri.SignalMastManager;
-import jmri.TurnoutManager;
-import jmri.jmrit.ctc.ctcserialdata.CodeButtonHandlerData;
 import jmri.jmrit.ctc.ctcserialdata.ProjectsCommonSubs;
 import jmri.jmrit.ctc.editor.gui.FrmMainForm;
 
@@ -26,12 +23,6 @@
  *
  */
 public class CheckJMRIObject {
-    private static final SensorManager SENSOR_MANAGER = InstanceManager.sensorManagerInstance();
-    private static final TurnoutManager TURNOUT_MANAGER = InstanceManager.turnoutManagerInstance(); //????
-    private static final SignalHeadManager SIGNAL_HEAD_MANAGER = InstanceManager.getDefault(jmri.SignalHeadManager.class);
-    private static final SignalMastManager SIGNAL_MAST_MANAGER = InstanceManager.getDefault(jmri.SignalMastManager.class);
-    private static final BlockManager BLOCK_MANAGER = InstanceManager.getDefault(BlockManager.class);
-    private static final FrmMainForm MAIN_FORM = InstanceManager.getDefault(FrmMainForm.class);
 
 //  Putting these strings ANYWHERE in a string variable definition (with EXACT case!)
 //  will cause this routine to try to validate it against JMRI Simple Server:
@@ -192,20 +183,20 @@ private VerifyClassReturnValue processField(String fieldName, String fieldConten
     }
 
     private boolean lowLevelCheck(OBJECT_TYPE objectType, String JMRIObjectName) {
-        if (!MAIN_FORM._mPanelLoaded) return true;
+        if (!InstanceManager.getDefault(FrmMainForm.class)._mPanelLoaded) return true;
         switch(objectType) {
             case SENSOR:
-                if (SENSOR_MANAGER.getSensor(JMRIObjectName) != null) return true;
+                if (InstanceManager.sensorManagerInstance().getSensor(JMRIObjectName) != null) return true;
                 break;
             case TURNOUT:
-                if (TURNOUT_MANAGER.getTurnout(JMRIObjectName) != null) return true;
+                if (InstanceManager.turnoutManagerInstance().getTurnout(JMRIObjectName) != null) return true;
                 break;
             case SIGNAL:
-                if (SIGNAL_HEAD_MANAGER.getSignalHead(JMRIObjectName) != null) return true; // Try BOTH:
-                if (SIGNAL_MAST_MANAGER.getSignalMast(JMRIObjectName) != null) return true;
+                if (InstanceManager.getDefault(SignalHeadManager.class).getSignalHead(JMRIObjectName) != null) return true; // Try BOTH:
+                if (InstanceManager.getDefault(SignalMastManager.class).getSignalMast(JMRIObjectName) != null) return true;
                 break;
             case BLOCK:
-                if (BLOCK_MANAGER.getBlock(JMRIObjectName) != null) return true;
+                if (InstanceManager.getDefault(BlockManager.class).getBlock(JMRIObjectName) != null) return true;
                 break;
             default:
                 break;

diff --git a/java/src/jmri/jmrit/ctc/editor/code/ProgramProperties.java b/java/src/jmri/jmrit/ctc/editor/code/ProgramProperties.java
@@ -5,16 +5,17 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Properties;
-import  jmri.util.FileUtil;
+import jmri.util.FileUtil;
 import jmri.jmrit.ctc.CTCFiles;
 
 /**
  *
  * @author Gregory J. Bedlek Copyright (C) 2018, 2019
  *
- * This just maintains all of the properties a user can create.  Stupid simple.
+ * This just maintains all of the properties a user can create. Stupid simple.
  */
 public class ProgramProperties {
+
     private static final String PROPERTIES_FILENAME = "ProgramProperties.xml";  // NOI18N
 
     public static final String FILENAME_DEFAULT = "preference:ctc/CTCSystem.xml";   // NOI18N
@@ -75,10 +76,10 @@ public class ProgramProperties {
     public ProgramProperties() {
         try {
             File file = CTCFiles.getFile(PROPERTIES_FILENAME);
-            FileInputStream fileInputStream = new FileInputStream(file);
             Properties properties = new Properties();
-            properties.loadFromXML(fileInputStream);
-            fileInputStream.close();    // Spotbugs complains, but "loadFromXML" has a "in.close()" and is DOCUMENTED to close it.  But it won't hurt.
+            try (FileInputStream fileInputStream = new FileInputStream(file)) {
+                properties.loadFromXML(fileInputStream);
+            }
 
             // Migrate file name if necessary
             String tempName = properties.getProperty(FILENAME_KEY, FILENAME_DEFAULT);
@@ -102,8 +103,7 @@ public ProgramProperties() {
             _mTUL_DispatcherInternalSensorLockTogglePattern = properties.getProperty(TUL_DISPATCHER_INTERNAL_SENSOR_LOCK_TOGGLE_PATTERN, TUL_DISPATCHER_INTERNAL_SENSOR_LOCK_TOGGLE_PATTERN_DEFAULT);
             _mTUL_DispatcherInternalSensorUnlockedIndicatorPattern = properties.getProperty(TUL_DISPATCHER_INTERNAL_SENSOR_UNLOCKED_INDICATOR_PATTERN, TUL_DISPATCHER_INTERNAL_SENSOR_UNLOCKED_INDICATOR_PATTERN_DEFAULT);
             _mCodeButtonDelayTime = Integer.parseInt(properties.getProperty(NO_CODE_BUTTON_DELAY_TIME_IN_MILLISECONDS, NO_CODE_BUTTON_DELAY_TIME_IN_MILLISECONDS_DEFAULT));
-        }
-        catch (IOException | NumberFormatException e) {
+        } catch (IOException | NumberFormatException e) {
             _mFilename = FileUtil.getExternalFilename(FILENAME_DEFAULT);
             _mCodeButtonInternalSensorPattern = CODE_BUTTON_INTERNAL_SENSOR_PATTERN_DEFAULT;
             _mSIDI_CodingTimeInMilliseconds = Integer.parseInt(SIDI_CODING_TIME_IN_MILLISECONDS_DEFAULT);
@@ -155,6 +155,7 @@ public void close() {
             try (FileOutputStream fileOutputStream = new FileOutputStream(file)) {
                 properties.storeToXML(fileOutputStream, PROPERTIES_FILENAME);
             }
-        } catch (IOException e) {}
+        } catch (IOException e) {
+        }
     }
 }