Skip to main content
added 1 character in body
Source Link
David Arno
  • 39.6k
  • 9
  • 95
  • 129

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 reoveIfImproperLengthremoveIfImproperLength(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;
}

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 reoveIfImproperLength(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;
}

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;
}
Post Undeleted by David Arno
Post Deleted by David Arno
Source Link
David Arno
  • 39.6k
  • 9
  • 95
  • 129

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 reoveIfImproperLength(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;
}