This is one of the less readable functions that I've found in my life. I've removed any reference to structures and variable names, according to the Italian law about privacy ;-)

I don't want to add any other comment. Enjoy!

static int my_function( /* Parameters */ )
{
  /* Some code */
  int j;
  for(j = 0; j < 2; j++) {
    /* Some code */
    if (j == 0) {
      /* Some code */
    } else {
      /* Some code */
    }
    /* Some code */
    if ( /* 1st condition */ ) {
      if ( /* Condition */ ) {
        /* Some code */
        if ( /* 2nd condition */ ) {
          /* Some code */
          if ( /* 3rd condition */ ) {
            /* Some code */
            if ( /* 4th condition */ ) {
              int i;
              /* Some code */
              for(i = 0; i < length; i++) {
                /* Some code */
                if ( /* 5th condition */ ) {
                  /* Some code */
                  if ( /* 6th condition */ ) {
                    if ( /* 7th condition */ ) {
                      /* Some code */
                      if ( /* 8th condition */ ) {
                        /* Some code */
                        if ( /* 9th condition */ ) {
                          /* Some code */
                          if ( /* 10th condition */ ) {
                            /* Some code */
                          }
                          /* Some code */
                          if ( /* Condition */ ) {
                            /* Some code */
                          } else {
                            /* Some code */
                          }
                        } else {
                            /* Some code */
                        }
                        /* Some code */
                      } else {
                        /* Some code */
                      }
                      /* Some code */
                    } else {
                      /* Some code */
                    }
                  } else {
                    /* Some code */
                  }
                } else {
                  /* Some code */
                }
              }
            } else {
              /* Some code */
            }
          } else {
            /* Some code */
          }
        } else {
          /* Some code */
        }
        /* Some code */
      } else {
        /* Some code */
      }
      /* Some code */
    }
  }
  /* Some code */
  if ( /* Condition */ ) {
    /* Some code */
  } else {
    /* Some code */
  }
  /* Some code */
  return 0;
}

The real function has 113 rows and most of the code in the else's is made of debug prints. The innermost code, the only one that actually does something, is indented 13 (thirteen) times.

Wasn't there a clearer way to write that function?

How to Fix It

Given that the return value is always 0, most of the if conditions can be reversed and the inner code be an early return, like in the following example:

  if ( ! /* 1st condition */ ) {
    /* Some code */
    return 0;
  }
  /* Some code */
  if ( ! /* 2nd condition */ ) {
    /* Some code */
    return 0;
  }
  ...

This approach might not be practical if there are several resources to freed before exiting the function. In this case, you can simply replace the return with a goto to the end of the function where all the de-allocations take place.

A detailed explanation on pros and cons of the return early pattern can be found on this article by Leonel Menaia.

Another thing that can be done to improve this code is to create a function with the code contained in the innermost for cycle. The structure of the code suggests that this part acts on single entities of some sort of collection; moving the operations on such entities in a separate function can for sure improve the code readability.

Post last updated on 2022/06/05.