Conditional logic to determine if popup menu should be visible

Posted on

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);
}

Leave a Reply

Your email address will not be published. Required fields are marked *