Return to Revisions

I really like how @dagnelies starts. Short and to the point. A good use of high level abstraction. I'm only really only tweaking it's signature (and avoiding a needless negative).

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

However, I like how @DavidArno breaks out the requirement tests as individual functions. Sure the whole thing becomes longer but every function is wonderfully tiny. I just don't like that they take on the extra responsibility of calling filterStringOut().

By the way, yes C++ will stop evaluation of a || chain on a true so long as you haven't overloaded the || operator. This is called short circuit evaluation.

bool isImproperString(QString s) {
    return isImproperLength(s, m_Length) 
        || sansRequiredSubstring(s, m_Include)
        || hasForbidenSubstring(s, m_Exclude)
    ;
}

Relieved of the need to call filterStringOut() the requirement functions become shorter and their names are much simpler.

bool isImproperLength(QString s, int length) {
    return ( s.length() != length );
}

bool sansRequiredSubstring(QString s, QStringList include) {
    for (int j=0; j<include.length(); j++) {
        QString requiredSubstring = include.at(j);
        if ( !s.contains(requiredSubstring) ) { 
            return true; 
        }
    }
    return false;
}

bool hasForbidenSubstring(QString s, QStringList exclude) {
    for (int j=0; j<exclude.length(); j++) {
    QString forbidenSubstring = exclude.at(j);
        if ( s.contains(forbidenSubstring) ) { 
            return true; 
        }
    }
    return false;
}

I added forbidenSubstring for the humans. Will it slow you down? Test and find out. It's easier to make readable code actually fast then to make prematurely optimized code readable or actually fast.

If the functions turn out to slow you down look into inline functions before you subject the humans to unreadable code. Again, don't assume this will give you speed. Test.

I think you'll find any of these more readable than nested for loops. Combined with the if's and you were starting to have a real arrow anti-pattern.