0

I have a file of a bunch of words and want to extract the ones that contain a 'q' or 'Q'. I have a function called isQWord that scans a string for these letters and, if it does contain these letters, puts the word into a vector called qwords. The function also does a few other things, but I've omitted these parts because they don't pertain to the issue. Then, in my main function, I use isQWord to read all the strings from my file. Here is my code:

bool isQWord(string str); // return true if str contains a 'q' or 'Q'

ifstream inFile;
vector<string> qwords;

int main() {

  string str;
  inFile.open("DictionaryLarge.txt");
  
  while(!inFile.eof()) { // go through contents of DictionaryLarge.txt
    inFile >> str;
    isQWord(str); // call isQWord function on file contents
  }

  inFile.close();
  outFile.close();
  
}


bool isQWord(string str) {
   
   /* This function does three things:
   1. scans a string for 'q' or 'Q'
   2. appends q-words to the qwords vector
   3. finds the largest q-word and stores all q-words before this word to an output file
   */
 
    
  for(int x=0; x < str.size(); x++) { // read string
    
    if(str.at(x) == 'q' || str.at(x) == 'Q') { // append to qwords vector for q-words
      qwords.push_back(str);
      return true;
    }
    
    else if(str.size()==0) { // return false for blanks
      return false;
    }
        
    else { // return false for non q-words
      return false;
    }
    
  }
}

However, the for loop in isQWord is "stuck", so to speak, at my starting value of x = 0 and only returning words that begin with 'q' or 'Q' only. That is, it's reading something like if(str.at(0) == 'q' || str.at(0) == 'Q') and completely bypassing the for loop.

I've also instead tried a while loop like this:



  int i=0;
  while(str.size() != 0) {
    if(str.at(i) == 'q' || str.at(i) == 'Q') {
      qwords.push_back(str);
      break;
    }
    
    i++;
  }

but this throws an error in my main function when I try to call isQWord on my file contents. I'm hoping to stick with the for loop because I think this is closer to working.

Also, I realize there are certain library functions I could be using that would make this easier, but this is a homework assignment and I'm only allowed to use .size() and .at() to search for 'q's and 'Q's.

Thank you for any guidance.

corgiworld
  • 37
  • 1
  • 7
  • Don't use `while(!inFile.eof())`. Simply `while(inFile >> str)` will be sufficient (_and have correct behavior_). Stylistically, `isQWord` is doing too much. It's also weird. It checks for zero string size inside a loop where that will never be true. It returns no value if a string is empty, and of course it has the bug you complained about where it returns if the first character is not a Q. It is also pushing stuff onto a vector. Don't make "test for thing" functions also do "change something else". Instead, use the return value and do it there instead. – paddy Oct 12 '20 at 05:02
  • Yes, I don't like the zero-string-size-inside-a-loop thing, either, but my professor wanted it :/. I have no idea why because obviously there aren't occurrences of this. I might just get rid of it entirely. Thanks for the help! – corgiworld Oct 12 '20 at 05:25
  • Usually this is the result of two possibilities: (A) you misunderstood your professor; (B) your professor is insane. I've seen many examples of both here on Stack Overflow, but Option A is by far the most common. – paddy Oct 12 '20 at 05:28
  • Well, he can be pretty unclear in his assignments, so yeah I probably misunderstand what he wants and will ask him. – corgiworld Oct 12 '20 at 05:37
  • This doesn't address the question, but with `for(int x = 0; x < str.size(); x++)` you **know** that `x` is a valid index into `str`. There's no need for the overhead of checking it again with `str.at(x)`. Just use `str[x]`. – Pete Becker Oct 12 '20 at 13:56
  • This also doesn't address the question, but that search can be written more simply as `if (str.find_first_of("qQ") != std::string::npos) { /* found a q or a Q */ }`. – Pete Becker Oct 12 '20 at 14:00
  • @PeteBecker these are great suggestions, thanks! The second one, I'm not familiar with that syntax, but I'll probably use it in the future. – corgiworld Oct 12 '20 at 19:51

1 Answers1

0

Inside your isQWord() function, you are breaking your for loop after the first iteration no matter whether the rest of the word contains a Q or not. You are not scanning that entire string. You need to move the return false statement after the loop ends without finding a Q.

You are putting too much responsibility on the function. It should simply return whether the word has a Q or not, nothing more. Let the caller decide what it do with the word based on the return value of the function.

Also, your use of inFile.eof() is just plain wrong. See Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?.

Try this instead:

bool isQWord(const string &str);

int main() {
  ifstream inFile;
  vector<string> qwords;

  string str;
  inFile.open("DictionaryLarge.txt");
  
  while (inFile >> str) {
    if (isQWord(str)) {
      qwords.push_back(str);
    }
  }

  return 0;
}


bool isQWord(const string &str) {
  for(size_t x = 0; x < str.size(); ++x) {
    if (str[x] == 'q' || str[x] == 'Q') {
      return true;
    }
  }
  return false;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thank you, I'm gonna go try these changes right now. About the while(!inFile.eof()), this is how my professor taught us to use .eof(), so I don't know what he likes about it :/ – corgiworld Oct 12 '20 at 05:14
  • @corgiworld if your professor doesn’t understand something basic like how eof() only works after an I/O operation is performed first, then you should find a better professor. Or maybe you simply misunderstood him to begin with. – Remy Lebeau Oct 12 '20 at 05:33
  • He told us to use while(!inFile.eof()) to go through files and has used it in a bunch of examples and whatnot. Thanks for bringing this to my attention. I'm going to look at the better way of doing it, the way you linked, and ask him about it. Maybe he'll realize this is a better method? – corgiworld Oct 12 '20 at 05:40
  • 1
    @corgiworld -- it's not really a question of whether this is a "better method"; `while (!inFile.eof())` is simply wrong. Yes, it wouldn't be diplomatic to say it that way, but don't lose sight of that basic fact. – Pete Becker Oct 12 '20 at 13:57
  • @PeteBecker now I'm curious to know why he's been using it so much and why he likes it if it's definitely wrong..... – corgiworld Oct 12 '20 at 19:55