current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

For this problem, I have to have two text files: one for list of words that need to be found and other containing the document that needs to be search. The results would placed in a output "results" file later on.

What I've Done: I've already figured out a way to store all of the word list onto an array and properly display it in the list.txt file.

Current Problem: How to find words that are the same and to increment the counter every time it is found. I've played around with the code but it still won't work.

Any help would be greatly appreciated!

int main()
{
  string wordList[100];
    string words;
    int wordCount = 0;

  inFile1.open("list.txt");
  if( inFile1.fail() )
  {
     cout << "Could not open input file - Aborting program!\n";
     exit(1);
  }
  else
  {
     for(int i = 0; i < 100; i++)
     {
        inFile1 >> wordList[i];
     }
  }

    outFile.open("results.txt");
    if( outFile.fail() )
    {
        cout << "Could not open output file - Aborting program!\n";
        exit(1);
    }

  inFile2.open("doc.txt");
    if( inFile2.fail() )
  {
     cout << "Could not open input file - Aborting program!\n";
     exit(1);
  }
    else
    {
        for (int i = 0; i < 100; i++)
        {
            while(inFile2 >> words)
            {
                if(words == wordList[i]) wordCount++;
                inFile2 >> words;
            }
        }
    }
outFile << "-----------------------" << endl;
    outFile << "Word\t\tCount" << endl;
    outFile << "-----------------------" << endl;
    for (int i = 0; i < 100; i++)
    {
        outFile.width(2);
        outFile << wordList[i] << "\t\t" << wordCount << endl;;
    }
  cout << words;



  inFile1.close();
  inFile2.close(); // Close all files explicitly
  outFile.close();
  return 0;  
}
share|improve this question

closed as off-topic by Hosch250, RubberDuck, rolfl Apr 17 at 19:01

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Hosch250, RubberDuck, rolfl
If this question can be reworded to fit the rules in the help center, please edit the question.
    
Welcome to Code Review! At Code Review, we only review working code, not code that does not work as expected. Once you get your code working as expected, be sure to come back so we can help you improve it! –  Hosch250 Apr 17 at 18:19

1 Answer 1

up vote 1 down vote

Your using string without specifying std:: which implies you have a using namespace std; in your code.

  string wordList[100];

Please try and avoid this. Its considered bad practice and one da will bight you in the ass. Get out of this habit. See Why is “using namespace std;” considered bad practice?

On the same line:

  string wordList[100];

Arrays are where you start. But usually it is best to use a standard container. There are a couple of pitfalls with arrays that you hit when you start to use functions. So prefer a std::vector over a raw array. (more on this below).

Thats long winded:

  if( inFile1.fail() )

You can test this with:

  if (!inFile1)    // This calls good(). (then add the not => !)
                   // Which is slightly better than fail()
                   // As there are other cases where you can't read from
                   // a file that don't put it in a fail state. good() means
                   // I can successfully read from it.

You are going to constantly be searching 'wordList' for a word. You don't say if the input file is sorted or anything. So searching is a always going to be a long presses as you need to go through each item in the array.

     for(int i = 0; i < 100; i++)
     {
        inFile1 >> wordList[i];
     }

If you had used a std::set this would keep your wordList in sorted order and thus do an optimized search that takes ln(n) comparisons to find your word (ie it takes 7 comparisons to search for a word rather than 100 comparisons).

So there are only 100 words in inFile2? I think you have your loops inside out.

        for (int i = 0; i < 100; i++)
        {
            while(inFile2 >> words)
            {

You should read a word. Then run through [0..100) loop to search for the value in the wordList.

You are only reading every second word:

            while(inFile2 >> words)
            {
                if(words == wordList[i]) wordCount++;
                inFile2 >> words;
            }

There is a read in the while condition: while(inFile2 >> words) and also inside the loop inFile2 >> words;. So you are ignoring every second word.

No need to explicitly close files.

  inFile1.close();
  inFile2.close(); // Close all files explicitly
  outFile.close();

See My C++ code involving an fstream failed review

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.