C++ Best Practices – How to Continue from Inside a Nested Loop

ccontinuationgotoloopsprogramming practices

Here is a simplified sample. Basically, it does checks on a string from a string list. If the check passes, it will remove that string (filterStringOut(i);), and it is no longer necessary to continue any other checks. Thus continue to the next string.

void ParsingTools::filterStrings(QStringList &sl)
{
    /* Filter string list */
    QString s;
    for (int i=0; i<sl.length(); i++) {
        s = sl.at(i);

        // Improper length, remove
        if (s.length() != m_Length) {
            filterStringOut(i);
            continue; // Once removed, can move on to the next string
        }          
        // Lacks a substring, remove
        for (int j=0; j<m_Include.length(); j++) {
            if (!s.contains(m_Include.at(j))) { 
                filterStringOut(i); 
                /* break; and continue; */ 
            }
        }
        // Contains a substring, remove
        for (int j=0; j<m_Exclude.length(); j++) {
            if (s.contains(m_Exclude.at(j))) { 
                filterStringOut(i); 
                /* break; and continue; */ 
            }
        } 
    }
}

How ought one continue the outer loop from inside a nested loop?

My best guess is to use goto and place a label at the end of the outer loop. That prompted me to ask this question, given how taboo goto can be.

In the c++ IRC chat, it was suggested that I place the for loops in bool functions, that return true if a check passed. thus

if ( containsExclude(s)) continue;
if (!containsInclude(s)) continue;

or that I simply create a local boolean, set it to true break, check bool and continue if true.

Given that I am using this in a parser, I actually do need to prioritize performance in this example. Is this a situation where goto is still useful, or is it a case where I need to restructure my code?

Best Answer

Don't nest: convert to functions instead. And have those functions return true if they perform their action and the subsequent steps can be skipped; false otherwise. That way you completely avoid the whole problem of how to break out of one level, continue within another, etc as you just chain the calls with || (this assumes C++ stops processing an expression on a true; I think it does).

So your code might end up looking like the following (I haven't written C++ in years, so it likely contains syntax errors, but should give you the general idea):

void ParsingTools::filterStrings(QStringList &sl)
{
    QString s;
    for (int i=0; i<sl.length(); i++) {
        s = sl.at(i);

        removeIfImproperLength(s, i) ||
        removeIfLacksRequiredSubstring(s, i) ||
        removeIfContainsInvalidSubstring(s, i);
    }
}

bool removeIfImproperLength(QString s, int i) {
    if (s.length() != m_Length) 
    {
        filterStringOut(i);
        return true;
    }
    return false;
}          

bool removeIfLacksSubstring(QString s, int i) {
    for (int j=0; j<m_Include.length(); j++) {
        if (!s.contains(m_Include.at(j))) { 
            filterStringOut(i);
            return true; 
        }
    }

    return false;
}

bool removeIfContainsInvalidSubstring(QString s, int i) {
    for (int j=0; j<m_Exclude.length(); j++) {
        if (s.contains(m_Exclude.at(j))) { 
            filterStringOut(i); 
            return true;
        }
    } 

    return false;
}
Related Topic