Is there a better way to write a nested if-else such as this?

Issue

Below is a snippet of code for a flutter function returning a widget. It either returns a button or text widget depending on four different variables.

Widget _renderReservationState() {
          if (reservationsDetailState.isFinished == false) {
            if (reservationsDetailState.rsvpStatus == "Attending") {
              if (reservationsDetailState.attendanceStatus == "Attended") {
                return Center(
                    child: Text("Registration completed");)
              } else {
                if (reservationsDetailState.isEditing == false) {
                  return SizedBox(
                      child: Button(
                          onPressed: () {
                            if (myReservationsDetailState.isEditing) {
                              _setEditMode(false);
                            } else {
                              _setEditMode(true);
                            }
                          },
                          title: "Edit reservation"),
                      width: double.infinity);
                } else {
                  return SizedBox(
                      child: Button(
                          onPressed: () {
                            if (reservationsDetailState.isEditing) {
                              _confirmReservation(dateController.text,
                                  timeController.text, attendeeController.text);
                            } else {
                              _setEditMode(true);
                            }
                          },
                          title: "Confirm reservation"),
                      width: double.infinity);
                }
              }
            } else {
              return SizedBox(
                  child: Button(
                      onPressed: () => {
                            _confirmReservation(dateController.text,
                                timeController.text, attendeeController.text),
                          },
                      title: "Make a reservation"),
                  width: double.infinity);
            }
          } else {
            if (myReservationsDetailState.attendanceStatus == "Attended") {
              return Center(
                child: Text("Thank you for your visit")
              );
            } else {
              return Center(
                  child: Text("Please join next time");)
            }
          }
        }

The existing code was a nested if-else over a couple hundred lines. I’ve refactored it to be shorter but I’m not happy with this implementation either. Any suggestions or recommendations would be appreciated.

Solution

Some general advice on improving readability:

• Get rid of unnecessary if checks:

  if (someCondition) {
    _setEditMode(false);
  } else {
    _setEditMode(true);
  }
  

  is equivalent to just:

  _setEditMode(!someCondition);
  

• Avoid if (condition == false). If condition is non-nullable, use if (!condition).

• Collapse nested if blocks into if–else if chains when possible:

  if (condition1) {
    ...
  } else {
    if (condition2) {
      ...
    } else {
      ...
    }
  }
  

  can be unindented a level to be:

  if (condition1) {
    ...
  } else if (condition2) {
    ...
  } else {
    ...
  }
  

• Don’t write top-heavy if–else blocks. Code like:

  if (condition1) {
    imagine();
    that();
    there();
    are();
    many();
    many();
    many();
    lines();
    ofCode();
    if (condition2) {
      withNested();
      blocks();
    } else {
      that();
      require();
      scrolling();
    }
  } else {
    someSingleLineOfCode();
  }
  

  is usually harder to follow than inverting the condition and putting the much shorter block on top:

  if (!condition1) {
    someSingleLineOfCode();
  } else {
    imagine();
    that();
    there();
    are();
    many();
    many();
    many();
    lines();
    ofCode();
    if (condition2) {
      withNested();
      blocks();
    } else {
      that();
      require();
      scrolling();
    }
  }
  

• Unindent else blocks by taking advantage of early exits:

  if (condition1) {
    return someWidget;
  } else {
    lots();
    of();
    other();
    code();
    return someOtherWidget;
  }
  

  can be:

  if (condition1) {
    return someWidget;
  }
  
  lots();
  of();
  other();
  code();
  return someOtherWidget;
  

• Try to emphasize which parts are the same across cases and which parts are different. Examples:

  if (myReservationsDetailState.attendanceStatus == "Attended") {
    return Center(child: Text("Thank you for your visit"));
  } else {
    return Center(child: Text("Please join next time"));
  }
  

  can become:

    return Center(
        child: Text(myReservationsDetailState.attendanceStatus == "Attended"
            ? "Thank you for your visit"
            : "Please join next time"));
  

  For the section with multiple, similar SizedBox(child: Button(...)) cases, I would create local variables to capture the differences and rearrange the code so that the common structure is shared.

Putting it all together:

Widget _renderReservationState() {
  if (reservationsDetailState.isFinished) {
    return Center(
        child: Text(myReservationsDetailState.attendanceStatus == "Attended"
            ? "Thank you for your visit"
            : "Please join next time"));
  }

  if (reservationsDetailState.rsvpStatus == "Attending" &&
      reservationsDetailState.attendanceStatus == "Attended") {
    return Center(child: Text("Registration completed"));
  }

  VoidCallback onButtonPressed;
  String buttonTitle;
  if (reservationsDetailState.rsvpStatus != "Attending") {
    onButtonPressed = () => _confirmReservation(
        dateController.text, timeController.text, attendeeController.text);
    buttonTitle = "Make a reservation";
  } else if (!reservationsDetailState.isEditing) {
    onButtonPressed = () => _setEditMode(!myReservationsDetailState.isEditing);
    buttonTitle = "Edit reservation";
  } else {
    onButtonPressed = () {
      if (reservationsDetailState.isEditing) {
        _confirmReservation(
            dateController.text, timeController.text, attendeeController.text);
      } else {
        _setEditMode(true);
      }
    };
    buttonTitle = "Confirm reservation";
  }

  return SizedBox(
      child: Button(onPressed: onButtonPressed, title: buttonTitle),
      width: double.infinity);
}

which is still ugly, but it has significantly less indentation, and in my opinion is better than it was originally.

Answered By – jamesdlin

Answer Checked By – Dawn Plyler (FlutterFixes Volunteer)