Skip to content

Commit

Permalink
spotbugs null checks, safe casting, access, null selection combos,
Browse files Browse the repository at this point in the history 
null params, matrixadd unused imports
  • Loading branch information
@silverailscolo
silverailscolo committed Jun 24, 2019
1 parent 587af33 commit 66fbf51
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 156 deletions.
7 changes: 4 additions & 3 deletions java/src/jmri/jmrit/beantable/SignalHeadTableAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2814,9 +2814,10 @@ protected Turnout updateTurnoutFromPanel(BeanSelectCreatePanel bp, String refere
if (oldTurnout == null || newTurnout == oldTurnout) {
return newTurnout;
}
if ((oldTurnout.getComment() != null) && (reference != null)) {
if (oldTurnout.getComment().equals(reference)) {
// won't delete old Turnout Comment if Locale or Bundle was changed in between
String oldComment = oldTurnout.getComment();
if ((oldComment != null) && (reference != null)) {
if (oldComment.equals(reference)) {
// won't delete old Turnout Comment if Locale or Bundle was changed since filling it in
// user could have typed something in the Comment as well
oldTurnout.setComment(null); // deletes current Comment in bean
}
Expand Down
15 changes: 9 additions & 6 deletions java/src/jmri/jmrit/beantable/TurnoutTableAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1206,13 +1206,16 @@ public static void updateAutomationBox(Turnout t, JComboBox<String> cb) {
}
if (t.getInhibitOperation()) {
cb.setSelectedIndex(0);
} else if (t.getTurnoutOperation() == null) {
cb.setSelectedIndex(1);
} else { // spotbugs happy null check
if (t.getTurnoutOperation().isNonce()) {
cb.setSelectedIndex(2);
} else {
TurnoutOperation turnOp = t.getTurnoutOperation();
if (turnOp == null) {
cb.setSelectedIndex(1);
} else {
cb.setSelectedItem(t.getTurnoutOperation().getName());
if (turnOp.isNonce()) {
cb.setSelectedIndex(2);
} else {
cb.setSelectedItem(turnOp.getName());
}
}
}
}
Expand Down
112 changes: 63 additions & 49 deletions java/src/jmri/jmrit/beantable/beanedit/TurnoutEditAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ public void actionPerformed(ActionEvent e) {
public void actionPerformed(ActionEvent e) {
Turnout t = (Turnout) bean;
String modeName = (String) modeBox.getSelectedItem();
t.setFeedbackMode(modeName);
if (modeName != null) {
t.setFeedbackMode(modeName);
}
String newName = operationsName.getText();
if ((currentOperation != null) && (newName != null) && !newName.isEmpty()) {
if (!currentOperation.rename(newName)) {
Expand All @@ -184,8 +186,11 @@ public void actionPerformed(ActionEvent e) {
break;
default: // named operation
t.setInhibitOperation(false);
t.setTurnoutOperation(InstanceManager.getDefault(TurnoutOperationManager.class).
getOperation(((String) automationBox.getSelectedItem())));
String autoMode = (String) automationBox.getSelectedItem();
if (autoMode != null) {
t.setTurnoutOperation(InstanceManager.getDefault(TurnoutOperationManager.class).
getOperation((autoMode)));
}
break;
}
oldAutomationSelection = ((Turnout) bean).getTurnoutOperation();
Expand All @@ -200,9 +205,6 @@ public void actionPerformed(ActionEvent e) {
} catch (jmri.JmriException ex) {
JOptionPane.showMessageDialog(null, ex.toString());
}
if (config.isEnabled()) {
// shouldn't there be some code here?
}
}
});

Expand Down Expand Up @@ -242,14 +244,16 @@ private void updateFeedbackOptions() {
sensorFeedBack1ComboBox.setEnabled(false);
sensorFeedBack2ComboBox.setEnabled(false);

if (modeBox.getSelectedItem().equals("ONESENSOR")) {
sensorFeedBack1ComboBox.setEnabled(true);
} else if (modeBox.getSelectedItem().equals("TWOSENSOR")) {
sensorFeedBack1ComboBox.setEnabled(true);
sensorFeedBack2ComboBox.setEnabled(true);
String mode = (String) modeBox.getSelectedItem();
if (mode != null) {
if (mode.equals("ONESENSOR")) {
sensorFeedBack1ComboBox.setEnabled(true);
} else if (mode.equals("TWOSENSOR")) {
sensorFeedBack1ComboBox.setEnabled(true);
sensorFeedBack2ComboBox.setEnabled(true);
}
t.setFeedbackMode(mode);
}

t.setFeedbackMode((String) modeBox.getSelectedItem());
jmri.jmrit.beantable.TurnoutTableAction.updateAutomationBox(t, automationBox);
}

Expand All @@ -258,8 +262,11 @@ private void updateAutomationOptions() {
currentOperation = null;
automationBox.removeActionListener(automationSelectionListener);
if (automationBox.getSelectedIndex() > 1) {
currentOperation = InstanceManager.getDefault(TurnoutOperationManager.class).
getOperation(((String) automationBox.getSelectedItem()));
String autoMode = (String) automationBox.getSelectedItem();
if (autoMode != null) {
currentOperation = InstanceManager.getDefault(TurnoutOperationManager.class).
getOperation((autoMode));
}
}

if (currentOperation != null) {
Expand Down Expand Up @@ -291,7 +298,7 @@ protected void cancelButtonAction(ActionEvent e) {
super.cancelButtonAction(e);
}

private final static String bothText = "Both";
private final static String bothText = "Both"; // TODO I18N using bundle. Note: check how this property is stored/loaded
private final static String cabOnlyText = "Cab only";
private final static String pushbutText = "Pushbutton only";
private final static String noneText = "None";
Expand Down Expand Up @@ -329,10 +336,13 @@ protected BeanItemPanel lock() {
lockOperationBox.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
if (lockOperationBox.getSelectedItem().equals(noneText)) {
lockBox.setEnabled(false);
} else {
lockBox.setEnabled(true);
String lockOp = (String) lockOperationBox.getSelectedItem();
if (lockOp != null) {
if (lockOp.equals(noneText)) {
lockBox.setEnabled(false);
} else {
lockBox.setEnabled(true);
}
}
}
});
Expand All @@ -346,25 +356,28 @@ public void actionPerformed(ActionEvent e) {
Bundle.getMessage("LockModeDecoder"),
Bundle.getMessage("LockModeDecoderToolTip")));

lock.setSaveItem(new AbstractAction() {
lock.setSaveItem(new AbstractAction() {
@Override
public void actionPerformed(ActionEvent e) {
Turnout t = (Turnout) bean;
String lockOpName = (String) lockOperationBox.getSelectedItem();
if (lockOpName.equals(bothText)) {
t.enableLockOperation(Turnout.CABLOCKOUT
+ Turnout.PUSHBUTTONLOCKOUT, true);
}
if (lockOpName.equals(cabOnlyText)) {
t.enableLockOperation(Turnout.CABLOCKOUT, true);
t.enableLockOperation(Turnout.PUSHBUTTONLOCKOUT, false);
}
if (lockOpName.equals(pushbutText)) {
t.enableLockOperation(Turnout.CABLOCKOUT, false);
t.enableLockOperation(Turnout.PUSHBUTTONLOCKOUT, true);
if (lockOpName != null) {
if (lockOpName.equals(bothText)) {
t.enableLockOperation(Turnout.CABLOCKOUT + Turnout.PUSHBUTTONLOCKOUT, true);
}
if (lockOpName.equals(cabOnlyText)) {
t.enableLockOperation(Turnout.CABLOCKOUT, true);
t.enableLockOperation(Turnout.PUSHBUTTONLOCKOUT, false);
}
if (lockOpName.equals(pushbutText)) {
t.enableLockOperation(Turnout.CABLOCKOUT, false);
t.enableLockOperation(Turnout.PUSHBUTTONLOCKOUT, true);
}
}
String decoderName = (String) lockBox.getSelectedItem();
t.setDecoderName(decoderName);
if (decoderName != null) {
t.setDecoderName(decoderName);
}
}
});

Expand Down Expand Up @@ -452,26 +465,26 @@ protected BeanItemPanel speed() {
public void actionPerformed(ActionEvent e) {
Turnout t = (Turnout) bean;
String speed = (String) closedSpeedBox.getSelectedItem();
try {
t.setStraightSpeed(speed);
if ((!speedListClosed.contains(speed))
&& !speed.contains("Global")) {
speedListClosed.add(speed);
if (speed != null) {
try {
t.setStraightSpeed(speed);
if ((!speedListClosed.contains(speed)) && !speed.contains("Global")) {
speedListClosed.add(speed);
}
} catch (jmri.JmriException ex) {
JOptionPane.showMessageDialog(null, ex.getMessage() + "\n" + speed);
}
} catch (jmri.JmriException ex) {
JOptionPane.showMessageDialog(null, ex.getMessage()
+ "\n" + speed);
}
speed = (String) thrownSpeedBox.getSelectedItem();
try {
t.setDivergingSpeed(speed);
if ((!speedListThrown.contains(speed))
&& !speed.contains("Global")) {
speedListThrown.add(speed);
if (speed != null) {
try {
t.setDivergingSpeed(speed);
if ((!speedListThrown.contains(speed)) && !speed.contains("Global")) {
speedListThrown.add(speed);
}
} catch (jmri.JmriException ex) {
JOptionPane.showMessageDialog(null, ex.getMessage() + "\n" + speed);
}
} catch (jmri.JmriException ex) {
JOptionPane.showMessageDialog(null,
ex.getMessage() + "\n" + speed);
}
}
});
Expand Down Expand Up @@ -506,4 +519,5 @@ public void actionPerformed(ActionEvent e) {
bei.add(speed);
return speed;
}

}
2 changes: 1 addition & 1 deletion java/src/jmri/jmrit/beantable/oblock/DnDJTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public boolean importData(JComponent comp, Transferable tr) {
try {
if (tr.isDataFlavorSupported(TABLECELL_FLAVOR)
|| tr.isDataFlavorSupported(DataFlavor.stringFlavor)) {
if (comp instanceof JTable) {
if (comp instanceof DnDJTable) {
DnDJTable table = (DnDJTable) comp;
AbstractTableModel model = (AbstractTableModel) table.getModel();
int col = table.getSelectedColumn();
Expand Down
41 changes: 23 additions & 18 deletions java/src/jmri/jmrit/beantable/signalmast/AddSignalMastPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.jdom2.Element;

/**
* JPanel to create a new Signal Mast
* JPanel to create a new Signal Mast.
*
* "Driver" refers to a particular class of SignalMast implementation that's to be configured.
*
Expand Down Expand Up @@ -184,12 +184,10 @@ public void itemStateChanged(ItemEvent e) {
updateSelectedDriver();
}
});


}

/**
* Select a particular signal implementation to display
* Select a particular signal implementation to display.
*/
void selection(String view) {
log.trace(" selection({}) start", view);
Expand All @@ -200,7 +198,7 @@ void selection(String view) {
}
}

// update that selected pane before display
// update that selected pane before display.
updateSelectedDriver();

// and show
Expand Down Expand Up @@ -276,8 +274,10 @@ public AddSignalMastPanel(SignalMast mast) {
ArrayList<File> mastFiles = new ArrayList<>(); // signal system definition files
LinkedHashMap<String, Integer> mapNameToShowSize = new LinkedHashMap<>();
LinkedHashMap<String, String> mapTypeToName = new LinkedHashMap<>();

// load the mast definitions from the selected signal system

/**
* Load the mast definitions from the selected signal system.
*/
void loadMastDefinitions() {
log.trace(" loadMastDefinitions() start");
// need to remove itemListener before addItem() or item event will occur
Expand Down Expand Up @@ -410,17 +410,20 @@ protected void updateSelectedDriver() {
currentPane.setMast(null);

currentPane.revalidate();
if (getTopLevelAncestor() != null && getTopLevelAncestor() instanceof Window) {
((JFrame)getTopLevelAncestor()).pack();

java.awt.Container ancestor = getTopLevelAncestor();
if ((ancestor instanceof JmriJFrame)) {
((JmriJFrame) ancestor).pack();
} else {
log.debug("Can't call pack() on {}", getTopLevelAncestor());
log.debug("Can't call pack() on {}", ancestor);
}
log.trace(" updateSelectedDriver() end");
}

/**
* Check of user name done when creating new SignalMast.
* In case of error, it looks a message and (if not headless) shows a dialog.
*
* @return true if OK to proceed
*/
boolean checkUserName(String nam) {
Expand Down Expand Up @@ -467,7 +470,7 @@ void issueWarningUserNameAsSystem(String nam) {
* <p>
* Invoked from Apply/Create button.
*/
void okPressed() {
private void okPressed() {
log.trace(" okPressed() start");
boolean success = false;

Expand All @@ -489,10 +492,10 @@ void okPressed() {

// ask top-most pane to make a signal
try {
success = currentPane.createMast(sigsysname,mastname,user);
success = currentPane.createMast(sigsysname, mastname, user);
} catch (RuntimeException ex) {
issueDialogFailMessage(ex);
return; // without clearing panel, so user can try again
return; // without clearing the panel, so user can try again
}
if (!success) {
// should have already provided user feedback via dialog
Expand Down Expand Up @@ -539,7 +542,7 @@ public void refresh() {
/**
* Respond to the Cancel button.
*/
void cancelPressed() {
private void cancelPressed() {
log.trace(" cancelPressed() start");
clearPanel();
log.trace(" cancelPressed() end");
Expand All @@ -550,17 +553,19 @@ void cancelPressed() {
* <p>
* Called at end of okPressed() and from Cancel
*/
void clearPanel() {
private void clearPanel() {
log.trace(" clearPanel() start");
if (getTopLevelAncestor() instanceof jmri.util.JmriJFrame) {
((jmri.util.JmriJFrame) getTopLevelAncestor()).dispose();
java.awt.Container ancestor = getTopLevelAncestor();
if ((ancestor instanceof JmriJFrame)) {
((JmriJFrame) ancestor).dispose();
} else {
log.warn("Unexpected top level ancestor: {}", getTopLevelAncestor()); // NOI18N
log.warn("Unexpected top level ancestor: {}", ancestor); // NOI18N
}
userName.setText(""); // clear user name
log.trace(" clearPanel() end");
}


private final static org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(AddSignalMastPanel.class);

}
Loading

0 comments on commit 66fbf51

Please sign in to comment.