using nested for loops to make code more efficient c++

Question

I am currently stuck on trying to nest several for loops. I will post the code and then explain it.

void IdeaBank::AND_searchQuery(string search_word1, string search_word2){

    vector <int> match;


    for (int i=0;i<newIdea.size();i++)
    { 
        for (int j=0;j<newIdea[i].getKeyword().size();j++)
        {

            if ( (newIdea[i].getKeyword()[0] == search_word1) && (newIdea[i].getKeyword()[j] == search_word2) )
            {
                match.push_back(newIdea[i].getID());
            }
            if ( (newIdea[i].getKeyword()[1] == search_word1) && (newIdea[i].getKeyword()[j] == search_word2) )
            {
                match.push_back(newIdea[i].getID());
            }
            if ( (newIdea[i].getKeyword()[2] == search_word1) && (newIdea[i].getKeyword()[j] == search_word2) )
            {
                match.push_back(newIdea[i].getID());
            }
        }
    }

    for (int i=0;i<match.size();i++)
    {
        displayIdeaByID(match[i]);
    }   
}

what the code above is doing and checking if two keywords appear in the same Idea and if it does to print that idea. the first loop goes over all my idea's, the second loop goes through all the keywords for 1 idea.

what i am trying to achieve is to see if both search words appear in the keywords for 1 idea.

the code above works correctly by checking all the keywords for a match however it is based on only 3 keywords. there are also n numbers of keywords inputted by a user.

I am trying to find a more efficient way to check if both search words are in one idea's keywords.

Read more about C++. Learn about standard containers and their time complexity. Provide full code, preferably an minimal reproducible example in your question — Basile Starynkevitch
– Basile Starynkevitch, Commented May 2, 2020 at 4:16

Tyler V · Accepted Answer · 2020-05-02 04:27:13Z

3

If you are searching for a case where both search terms are in the keyword list for a given item, you could just use std::find instead of the inner loop. For example:

for (auto&& idea : newIdea)
{ 
    const auto& keywords = idea.getKeywords();
    bool found1 = std::find(keywords.begin(), keywords.end(), search_word1) != keywords.end();
    bool found2 = std::find(keywords.begin(), keywords.end(), search_word2) != keywords.end();

    if( found1 && found2 ) {
        match.push_back(idea.getID());
    }
}

edited May 2, 2020 at 4:27

answered May 2, 2020 at 4:16

Tyler V

11.2k4 gold badges36 silver badges60 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Some programmer dude Over a year ago

Personally I would use auto const& keywords = idea.getKeywords(); instead. And put in a const somewhere for idea as well (and probably use plain reference instead of rvalue reference).

Tyler V Over a year ago

Fair point, although given the snippet of code I have no idea whether getKeywords and getID are const methods.

william_ Over a year ago

are you saying to just have one loop that goes through all my ideas and inside that loop add your code?

Tyler V Over a year ago

@william_ This would replace both the loops (over ideas then over keywords) with this one loop over ideas. Loop over ideas and use std::find to see if the keywords are both present.

william_ Over a year ago

not sure why i had to keep the first loop and then move the if statement outside of the auto loop and that worked for me so thankyou

Collectives™ on Stack Overflow

using nested for loops to make code more efficient c++

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related