Problem
I have this small piece of code written in a class. I am looking for a nicer way to write this, preferably avoiding nested if
statements like what I currently have:
@Override
public void popupMenuWillBecomeVisible(PopupMenuEvent e)
{
TheatreListDataWrapper[] selectedOperations = getSelectedDatas();
boolean isOperationFromMulitpleSoc = TheatreListDataWrapper.operationsOfMultiplePatients(selectedOperations);
if (!isOperationFromMulitpleSoc)
{
//passing boolean literal value 'false' instead of variable 'isOperationFromMultipleSoc' since that,
//at this point it is certain that the operations selected are from a single patient
CambioAction attentionSignalAction = AttentionSignalDelegator.getAttentionSignalAction(false,selectedOperations);
if (attentionSignalAction != null)
{
if (Framework.getInstance().getActiveSubjectOfCare().id == null
|| !getAcuteListTable().getTableDataByRow(getAcuteListTable().getSelectedRow()).socId.equals(Framework
.getInstance().getActiveSubjectOfCare().id))
{
attentionSignalAction.setEnabled(false);
}
else
{
attentionSignalAction.setEnabled(true);
}
}
}
if(selectedOperations != null && selectedOperations.length > 1){
listRegistraionOpenMenuItem.setEnabled(false);
}else{
listRegistraionOpenMenuItem.setEnabled(true);
}
}
For me, using that last if checking seems like, I could do it in a better way, using a switch or some other control structure. Is there a way to avoid the null
checks?
Solution
You may not need another if else to setEnabled. The expression within if returns a boolean. Just set the inverse of it to setEnabled. You can pull it into a variable in case of better readability.
Updated:
if (attentionSignalAction != null) {
bool isActiveRowSelected = (Framework.getInstance().getActiveSubjectOfCare().id == null||!getAcuteListTable().getTableDataByRow(getAcuteListTable().getSelectedRow()).socId.equals(Framework.getInstance().getActiveSubjectOfCare().id));
attentionSignalAction.setEnabled(!isActiveRowSelected);
}
Update 2
if(selectedOperations != null && selectedOperations.length > 1){
listRegistraionOpenMenuItem.setEnabled(false);
}else{
listRegistraionOpenMenuItem.setEnabled(true);
}
can also be reduced to
bool operationsSelected = (selectedOperations != null && selectedOperations.length > 1);
listRegistraionOpenMenuItem.setEnabled(!operationsSelected);
boolean
expressions
Instead of a mere negation of your boolean
evaluation, you can express the literal inversion:
// typo? listRegistraionOpenMenuItem -> listRegistrationOpenMenuItem
listRegistrationOpenMenuItem.setEnabled(
selectedOperations == null || selectedOperations.length <= 1);
The null
check for Framework.getInstance().getActiveSubjectOfCare().id
is not required, as the equals()
method for any class should return false
for null
. Hence, in a similar manner, enabling attentionSignalAction
can be done as such:
// T is a placeholder for the actual type
T row = getAcuteListTable().getTableDataByRow(getAcuteListTable().getSelectedRow());
T activeSubject = Framework.getInstance().getActiveSubjectOfCare();
attentionSignalAction.setEnabled(!row.socId.equals(activeSubject.id));
Of course, this assumes that the type of socId
is handling null
comparison safely (by returning false
), instead of throwing a RuntimeException
.
If you happen to be on Java 8, the Optional
class provides a wrapper interface to handle null
-checks in an arguably more fluent way.
Putting it altogether
@Override
public void popupMenuWillBecomeVisible(PopupMenuEvent e)
{
TheatreListDataWrapper[] selected = getSelectedDatas();
if (!TheatreListDataWrapper.operationsOfMultiplePatients(selected))
{
Optional.of(AttentionSignalDelegator.getAttentionSignalAction(false, selected))
.ifPresent(v ->
{
T row = getAcuteListTable()
.getTableDataByRow(getAcuteListTable().getSelectedRow());
T activeSubject = Framework.getInstance().getActiveSubjectOfCare();
v.setEnabled(!row.socId.equals(activeSubject.id));
});
}
listRegistrationOpenMenuItem.setEnabled(selected == null || selected.length <= 1);
}