Is there a better way to write this code?

I really want to start learning how to wield the string class and using it to make games and programs and stuff, but before i get too far, i created a small program that searches for a string, is it the most efficient way to write it?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
#include <iostream>
#include <string>

using namespace std;

int main()
{
    string stringToSearch = "This is a string of text to search.";
    string stringToFind{};

    cout << stringToSearch << endl;

    cout << "Enter a string to find" << endl;

    getline(cin, stringToFind);

    unsigned int wordAppearance = 0;

    for(unsigned int position = stringToSearch.find(stringToFind, 0); position != string::npos; position = stringToSearch.find(stringToFind, position))
    {
        cout << "Found " << ++wordAppearance << " instances of " << stringToFind << " at position " << position << endl;
        position++;
    }

    return 0;
}
Last edited on
What happens if it DOESN'T find the string?

Change to a while() loop so that it will test the condition at the top of the loop, not the end.
Last edited on
Something like this? i'm a little confused, i'm trying to convert the for loop to the while loop but it keeps looping infinitly because i believe it has something to do with string never being a new position.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
#include <iostream>
#include <string>

using namespace std;

int main()
{
    string stringToSearch = "This is a string of text to search.";
    string stringToFind{};

    cout << stringToSearch << endl;

    cout << "Enter a string to find" << endl;

    getline(cin, stringToFind);

    unsigned int wordAppearance{};

    unsigned int position = stringToSearch.find(stringToFind, 0);

    while(position != string::npos)
    {
        cout << "Found " << ++wordAppearance << " instances of " << stringToFind << " at position " << position << endl;

        position++;
    }
/*
    for(unsigned int position = stringToSearch.find(stringToFind, 0); position != string::npos; position = stringToSearch.find(stringToFind, position))
    {
        cout << "Found " << ++wordAppearance << " instances of " << stringToFind << " at position " << position << endl;
    }
*/
    return 0;
}
My apologies, @Ch1156, I missed your updated thread.

Actually, the logic was more tortuous than I realised. The following works (I think) but there are probably better ways. I'm not fond of having multiple logic tests per loop.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
#include <iostream>
#include <string>
using namespace std;

int main()
{
    string stringToSearch = "This is a string of text to search.";
    cout << stringToSearch << endl;

    string stringToFind{};
    cout << "Enter a string to find: ";
    getline(cin, stringToFind);

    unsigned int wordAppearance = 0;
    int position = 0;       // Edited
    while( position < stringToSearch.size() )
    {
       position = stringToSearch.find( stringToFind, position );
       if ( position == string::npos ) break;

       cout << "Found instance number " << ++wordAppearance << " of " << stringToFind << " at position " << position << endl;
       position++;         // move forward or it finds the same thing again
    }

    if ( !wordAppearance ) cout << stringToFind << " not found ";
}
Last edited on
> What happens if it DOESN'T find the string?
the program terminates without printing to the console.
¿is there any problem with that?
> What happens if it DOESN'T find the string?
the program terminates without printing to the console.
¿is there any problem with that?


Oh I don't have problems with programs writing nothing.
I do have problems when I try running it in C++ shell and it cripples my web browser by printing out line after line of garbage.

for(unsigned int position = stringToSearch.find(stringToFind, 0); ...

I think that in the absence of finding the requisite string this ought to set position to string::npos ... and then proceed to enter the loop ...

... or perhaps not, if position is unsigned and string::npos comes back as -1
Last edited on
I think that in the absence of finding the requisite string this ought to set position to string::npos ... and then proceed to enter the loop ...

No, the for loop acts like a while so if the condition is not satisfied it would not enter the loop at all.

... or perhaps not, if position is unsigned and string::npos comes back as -1

OP's code simply doesn't work because he's using unsigned int instead of size_t.
Basically the loop will never end because pos is unsigned and will never be -1.
I think I fell into the same trap! Just edited to remove "unsigned" from my own code.

I mistakenly thought that the middle condition was first tested at the end of the first loop. Sorry! Back to basics for me!
Last edited on
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
#include <iostream>
#include <string>

int main()
{
    std::string stringToSearch { "This is a string of text to search." };
    std::cout << stringToSearch << '\n';
    std::string stringToFind { "is" };

    unsigned wordAppearance {1};
    for(std::string::size_type position { stringToSearch.find(stringToFind) };
        position != std::string::npos;
        position = stringToSearch.find(stringToFind, position+1))
    {
        std::cout << "Match " << wordAppearance++ << ": instance of " 
                  << stringToFind << " found at position " << position << '\n';
    }
    return 0;
}


This is a string of text to search.
Match 1: instance of is found at position 2
Match 2: instance of is found at position 5

closed account (SECMoG1T)
maybe you could just use while loops, i think they are quite simple for your case.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30

#include <iostream>

void to_search(const std::string& , const std::string& );


int main()
{
   auto data   = "mango juice,milk,sugar,lemon juice,potato,lettuce,orange juice,banana juice";
   auto phrase = "juice";

   to_search(data,phrase);
}


void to_search(const std::string& here, const std::string& stringToFind)
{
    std::size_t position{},wordAppearance{} ;

    while((position = here.find(stringToFind,position)) != std::string::npos)
    {
      std::cout << "Match " << ++wordAppearance << ": instance of " << stringToFind << " found at position " << position << '\n';
      position += stringToFind.size();
    }

    if(wordAppearance == 0)
        std::cout<<"the string \'"<<stringToFind<<"\' was not found\n";
}





Match 1: instance of juice found at position 6
Match 2: instance of juice found at position 29
Match 3: instance of juice found at position 57
Match 4: instance of juice found at position 70
Last edited on
Sorry for the late reply, thank you for your responses. I thought that size_t was the same thing as unsigned int but reading up on it, I found out that it wasnt.
Topic archived. No new replies allowed.