Doug Ross @ Journal

Software Development Best Practices: Minimizing Nesting

I'm going to spend a little time blogging about my personal software development best practices. These posts will come in no particular order, but will outline the rules I like to follow when developing software. Reliability, maintainability and simplicity are my personal mantras for development.

Minimizing Nesting

Unncessary nesting is, if not evil, pretty darn annoying. Whenever possible, developers should strive to minimize nesting. Why? Let's say I have the following code:

// void CEventHandler::OnNew(const CString& strFileName) {   //   CString      strName = CleanFilename(strFileName);   CString      strLog;   //   do {     //  File-type valid? If not, quit.     //     if (!IsFileTypeValid(strName)) {       break;     }     //  Mark file as added.     //     m_pPage->m_mapFileClassification.SetAt(strName, (LPVOID) XMP_FILE_ADD);     m_pPage->ScheduleRefresh();     //  Event handling.     //     strLog.Format("Added: '%s%s'", m_pPage->m_strPath, strName);     if ((m_pPage->m_folderSettings.m_dwEvents & CFolderSettings::FileCreated) != 0) {       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionLog) != 0) {         Log(strLog);       }       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionRoute) != 0) {         m_pPage->ScheduleRoute();       }       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionEmail) != 0) {         m_pPage->ScheduleEmail();       }     }   //   } while (0); }

Note that I could have made the IsFileType a nested IF clause: if the file type is valid, then do all the rest of the stuff. But I didn't. Because (and this really happened), I later realized that it was in the wrong place. The file-type validity check needed to occur after the scheduled (display) refresh embodied by ScheduleRefresh. So I simply moved those lines of code:

// void CEventHandler::OnNew(const CString& strFileName) {   //   CString      strName = CleanFilename(strFileName);   CString      strLog;   //   do {     //  Mark file as added.     //     m_pPage->m_mapFileClassification.SetAt(strName, (LPVOID) XMP_FILE_ADD);     m_pPage->ScheduleRefresh();     //  File-type valid? If not, quit.     //     if (!IsFileTypeValid(strName)) {       break;     }     //  Event handling.     //     strLog.Format("Added: '%s%s'", m_pPage->m_strPath, strName);     if ((m_pPage->m_folderSettings.m_dwEvents & CFolderSettings::FileCreated) != 0) {       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionLog) != 0) {         Log(strLog);       }       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionRoute) != 0) {         m_pPage->ScheduleRoute();       }       if ((m_pPage->m_folderSettings.m_dwActions & CFolderSettings::ActionEmail) != 0) {         m_pPage->ScheduleEmail();       }     }   //   } while (0); }

Now, if I'd originally nested the IF, I'd have to move a bunch more code around... shifting a lot of indentation... and, in general, opening the door to possible errors.

Minimizing nesting greatly aids readability and, thus, maintainability. Whenever possible, bail out of logic (break, throw an exception, whatever floats your boat) when you can cleanly exit a method or function. Don't nest when you don't have to.