What is wrong with my logic? Trying to count amount of words in a file that contain a specific character

Question

First time posting a question. Seeking debugging help. I am trying to figure out where my logic is wrong in a c++11 function. The goal is to search for a given character in a file and record the number words it is in, the number of lines it is in, and the number of times it appears in the file in total. Lines and total amount of occurrences are accurate but the word count is off which I assume is because of my logic. lwc is a class I created and has int set methods. Where am I going wrong to properly count the amount of words that has a specific character in it? Thanks for your time.

void findChar(lwc *CH, string File)
{
ifstream file(File);
int a=0;
int b=0;
int c=0;
string line;
bool i = false;
bool j = false;
char ch;
while (!file.eof())
{

        getline(file, line);
        for (int g=0; g<line.length(); g++)
        {       ch = line[g];
                if (ch == CH->getChar()) //to count characters
                {a++; i= true; j = true;}

                if (isspace(ch) && j) //to count words
                {c++; j=false;}
        }
        if (i)                 //to count lines
        {b++; i = false;}

}

CH->setWords(c);
CH->setChars(a);
CH->setLines(b);

}

Please read [Why is iostream::eof inside a loop condition considered wrong?](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). — Some programmer dude, Mar 07 '17 at 22:37
Is it possible that the word appears at the end of the line, and therefore is followed by a newline character instead of a space? — heyiamt, Mar 07 '17 at 22:40
To start with, writing more than one line of code per line is just bad. — Barry, Mar 07 '17 at 22:41
Free tip: use meaningful variable names instead of `i`, `j` etc. `i` and `j` are typically used as counters for loops (kind of an unwritten "rule") - if you gave them meaningful names like `found_char_on_line` and `found_char_in_current_word` your code you be much clearer. — John3136, Mar 07 '17 at 22:45
T- \n \t \r are included in isspace(). The problem I think was, (maybe) when I grab a line and assign it to a string, they are not included in the string. — Zman25, Mar 08 '17 at 16:43

score 0 · Answer 1 · answered Mar 07 '17 at 22:44

while (!file.eof())

This is already wrong, as it's being pointed out already. The effects of this bug are mostly mitigated, due to the following getline(), which will simply read an empty string, on the erroneous iteration, but it's still a bug, and you should fix it.

But your main bug is here:

   if (ch == CH->getChar()) //to count characters
   {a++; i= true; j = true;}

   if (isspace(ch) && j) //to count words
   {c++; j=false;}

This relies on every word being followed by a space, in order to reset j to false. You are using j as a flag that gets set when the character is found as part of the word, so when a space that follows a word is encountered, c gets incremented (the number of words your character appears in), and j gets reset.

However, you'll be shocked to learn that not every word in your file will be followed by a space. The last word on each line will not have a space after it (unless there's an explicit trailing space at the end of the line). Surprise!

Thusly, you will finish the current line's iteration with j remaining set to true. When the loop iterates again, on the next line, j will remain set to true, and your logic will then erroneously conclude that your character occured somewhere in the first word of the next line, when it really didn't.

Ali Asadpoor · Answer 2 · 2017-03-08T17:04:49.347

0

You assume that all words will end with space, but it is possible for a word at the end of a line not to have trailing space. if ((isspace(ch) || g == line.length() - 1) && j ) //to count characters

edited Mar 08 '17 at 17:04

answered Mar 07 '17 at 22:44

Ali Asadpoor

317
1
12

Wouldn't this set i and j to true if it reaches the end of a line? If it reaches the end of the line and does not find the character, i and j would still be set to true, which is an error (correct me if I'm wrong please). But taking what you said into consideration, I think I found an answer – Zman25 Mar 08 '17 at 16:59

score 0 · Answer 3 · answered Mar 08 '17 at 17:06

Taking your comments and answers into consideration this is what I came up with. It gave the correct output in my tests. I took out file.eof(), bumped getline(file,line) up to be in the while statement, and added line.length()-1 to accommodate for the end of the string not having a space character.

//while (!file.eof())
while (getline(file,line))
{

    //getline(file, line);
    for (int g=0; g<line.length(); g++)
    {       ch = line[g];
            if (ch == CH->getChar()) //to count characters
            {a++; i= true; j = true;}  

            if (isspace(ch) || g == line.length()-1) // to count words
            {
                    if(j)
                    {c++;j=false;}
            }
    }
    if (i)          //to count lines
    {b++; i = false;}

}

What is wrong with my logic? Trying to count amount of words in a file that contain a specific character

3 Answers3