Praktik terbaik untuk "melanjutkan" dari dalam lingkaran bersarang?

Berikut adalah contoh yang disederhanakan. Pada dasarnya, ia memeriksa string dari daftar string. Jika cek lolos, itu akan menghapus string itu ( filterStringOut(i);), dan tidak perlu lagi melanjutkan pemeriksaan lainnya. Demikian continueke string selanjutnya.

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; */ 
            }
        } 
    }
}

Bagaimana seharusnya seseorang melanjutkan loop luar dari dalam loop bersarang?

Tebakan terbaik saya adalah menggunakan gotodan menempatkan label di ujung lingkaran luar. Itu mendorong saya untuk mengajukan pertanyaan ini, mengingat bagaimana hal itu gotobisa tabu .

Dalam obrolan c ++ IRC, disarankan agar saya menempatkan forloop dalam fungsi bool, yang mengembalikan true jika tanda centang dilewati. jadi

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

atau bahwa saya cukup membuat boolean lokal, atur menjadi true break, periksa bool dan lanjutkan jika benar.

Mengingat bahwa saya menggunakan ini dalam parser, saya sebenarnya perlu memprioritaskan kinerja dalam contoh ini. Apakah ini situasi di mana gotomasih berguna, atau ini merupakan kasus di mana saya perlu merestrukturisasi kode saya?

Jangan bersarang: konversikan ke fungsi saja. Dan minta fungsi-fungsi itu kembali truejika mereka melakukan tindakan mereka dan langkah-langkah selanjutnya dapat dilewati; falsejika tidak. Dengan cara itu Anda benar-benar menghindari seluruh masalah bagaimana keluar dari satu level, melanjutkan di dalam level lain, dll ketika Anda hanya menghubungkan panggilan dengan ||(ini mengasumsikan C ++ berhenti memproses ekspresi pada true; Saya pikir itu).

Jadi kode Anda mungkin berakhir seperti berikut ini (saya belum menulis C ++ selama bertahun-tahun, sehingga kemungkinan mengandung kesalahan sintaks, tetapi seharusnya memberi Anda ide umum):

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

Dari sudut pandang lebih banyak burung, saya akan refactor kode sehingga terlihat seperti ini ... (dalam kode semu, sudah terlalu lama saya menyentuh C ++)

void filterStrings(sl)
{
    /* Filter string list */
    for (int i=0; i<sl.length(); i++) {
        QString s = sl.at(i);
        if(!isProperString(s)) {
           filterStringOut(i);
        }
     }
}

bool isProperString(s) {

        if (s.length() != m_Length)
            return false; // Improper length

        for (int j=0; j<m_Include.length(); j++) {
            if (!s.contains(m_Include.at(j))) { 
                return false; // Lacks a substring
            }
        }

        for (int j=0; j<m_Exclude.length(); j++) {
            if (s.contains(m_Exclude.at(j))) { 
                return false; // Contains a substring
            }
        }

        return true; // all tests passed, it's a proper string
}

Ini adalah IMHO cleaner karena dengan jelas memisahkan apa yang merupakan string yang tepat, dan apa yang Anda lakukan saat tidak.

Anda bahkan dapat melangkah lebih jauh dan menggunakan metode filter bawaan seperti myProperStrings = allMyStrings.filter(isProperString)

Saya sangat suka bagaimana @agnagnies dimulai . Singkat dan langsung ke inti nya. Penggunaan abstraksi tingkat tinggi yang baik. Saya hanya mengubah tanda tangan dan menghindari hal-hal negatif yang tidak perlu.

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

Namun, saya suka bagaimana @DavidArno memecah tes persyaratan sebagai fungsi individual. Tentu semuanya menjadi lebih panjang tetapi setiap fungsi luar biasa kecil. Nama mereka menghindari perlunya komentar untuk menjelaskan siapa mereka. Saya hanya tidak suka mereka mengambil tanggung jawab ekstra untuk menelepon filterStringOut().

Omong-omong, ya C ++ akan menghentikan evaluasi ||rantai trueselama Anda belum membebani ||operator. Ini disebut evaluasi hubung singkat . Tapi ini adalah optimasi mikro sepele yang Anda bebas abaikan ketika Anda membaca kode selama fungsinya bebas efek samping (seperti yang di bawah).

Yang berikut harus membuat definisi string tolak menjadi jelas tanpa menyeret Anda melalui detail yang tidak perlu:

bool isRejectString(QString s) {
    return isDifferentLength(s, m_Length) 
        || sansRequiredSubstring(s, m_Include)
        || hasForbiddenSubstring(s, m_Exclude)
    ;
}

Lega dari kebutuhan untuk memanggil filterStringOut()fungsi tes persyaratan menjadi lebih pendek dan nama mereka jauh lebih sederhana. Saya juga menaruh semua yang mereka andalkan dalam daftar parameter mereka untuk membuatnya lebih mudah untuk memahaminya tanpa melihat ke dalam.

bool isDifferentLength(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 hasForbiddenSubstring(QString s, QStringList &exclude) {
    for (int j=0; j<exclude.length(); j++) {
    QString forbiddenSubstring = exclude.at(j);
        if ( s.contains(forbiddenSubstring) ) { 
            return true; 
        }
    }
    return false;
}

Saya menambahkan requiredSubstringdan forbiddenSubstringuntuk manusia. Apakah mereka akan memperlambat Anda? Tes dan temukan. Lebih mudah untuk membuat kode yang dapat dibaca benar-benar cepat daripada membuat kode yang dioptimalkan secara prematur dapat dibaca atau benar-benar cepat.

Jika fungsi ternyata memperlambat Anda lihatlah ke fungsi sebaris sebelum Anda membuat manusia kode yang tidak dapat dibaca. Sekali lagi, jangan menganggap ini akan memberi Anda kecepatan. Uji.

Saya pikir Anda akan menemukan ini lebih mudah dibaca daripada bersarang untuk loop. Mereka, dikombinasikan dengan if's, mulai memberi Anda panah anti-pola nyata . Saya pikir pelajarannya di sini adalah bahwa fungsi kecil adalah hal yang baik.

Cukup gunakan lambda untuk predikat, dan kemudian gunakan kekuatan algoritma standar dan hubungan arus pendek. Tidak perlu untuk aliran kontrol berbelit-belit atau eksotis:

void ParsingTools::filterStrings (QStringList& list)
{
    for (int i = list.size(); i--;) {
        const auto& s = list[i];
        auto contains = [&](const QString& x) { return s.contains(x); };
        if (s.size() != m_Length
                || !std::all_of(m_Include.begin(), m_Include.end(), contains)
                || std::any_of(m_Exclude.begin(), m_Exclude.end(), contains))
            filterStringOut(i);
    }
}

Ada juga opsi untuk membuat konten dari loop luar (yang Anda ingin melanjutkan) lambda , dan cukup gunakan return.
Sangat mudah jika Anda tahu lambdas; Anda pada dasarnya memulai interior lingkaran Anda dengan [&]{dan mengakhirinya dengan }(); di dalam Anda dapat menggunakan return;kapan saja untuk meninggalkannya:

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

      [&]{    // start a lamdba defintion

        s = sl.at(i);

        // Improper length, remove
        if (s.length() != m_Length) {
            filterStringOut(i);
            // continue; // Once removed, can move on to the next string
            return; // happily return here, this will continue 
        }          
        // 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; */  return;  // happily return here, this will continue the i-loop
            }
        }
        // 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; */  return; // happily return here, this will continue the i-loop
            }
        } 

      }()   // close/end the lambda definition and call it

    }
}

Saya pikir @dganelies memiliki ide yang tepat sebagai titik awal, tapi saya pikir saya akan mempertimbangkan untuk melangkah lebih jauh: menulis fungsi generik yang dapat menjalankan pola yang sama untuk (hampir) wadah, kriteria, dan tindakan apa pun:

template <class Container, class Action, class Condition>
void map_if(Container &container, Action action, Condition cond) {
    for (std::size_t i = 0; i < container.length(); i++) {
        auto s = container.at(i);
        if (cond(s))
            action(i);
    }
}

Anda filterStringskemudian hanya akan menentukan kriteria, dan lulus tindakan yang sesuai:

void ParsingTools::filterStrings(QStringList const &sl)
{
    auto isBad = [&](QString const &s) {

        if (s.length() != m_Length)
            return true;

        for (int j = 0; j < m_Include.length(); j++) {
            if (!s.contains(m_Include.at(j))) {
                return true;
            }
        }

        for (int j = 0; j < m_Exclude.length(); j++) {
            if (s.contains(m_Exclude.at(j))) {
                return true;
            }
        }
        return false;
    };

    map_if(sl, filterStringOut, isBad);
}

Tentu saja, ada cara lain untuk mendekati masalah mendasar itu juga. Misalnya, menggunakan perpustakaan standar, Anda sepertinya menginginkan sesuatu dengan urutan umum yang sama std::remove_if.

Beberapa jawaban menyarankan refactor utama kode. Ini mungkin bukan cara yang buruk untuk pergi, tetapi saya ingin memberikan jawaban yang lebih sesuai dengan pertanyaan itu sendiri.

Aturan # 1: Profil sebelum dioptimalkan

Selalu profil hasil sebelum mencoba optimasi. Anda mungkin menghabiskan banyak waktu jika tidak melakukannya.

Yang telah dibilang...

Seperti itu, saya secara pribadi telah menguji kode semacam ini di MSVC. Boolean adalah cara untuk pergi. Beri nama boolean sesuatu yang semantik bermakna seperti containsString.

    ...
    boo containsString = true; // true until proven false
    // 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; */ 
            containsString = false;
        }
    }
    if (!containsString)
        continue;

Pada MSVC (2008), dalam mode rilis (pengaturan pengoptimal khas), kompiler mengoptimalkan loop yang sama ke persis set yang sama dari opcodes seperti goto versi. Cukup cerdas untuk melihat bahwa nilai boolean secara langsung terkait dengan kontrol aliran, dan menyingkirkan semuanya. Saya belum menguji gcc, tapi saya kira itu bisa melakukan optimasi serupa.

Ini memiliki keunggulan dibandingkan gototidak hanya menimbulkan kekhawatiran oleh kaum puritan yang dianggap gotoberbahaya, tanpa mengorbankan nilai kinerja instruksi tunggal.