My character counting code is miscounting characters. Why?

Question

The input file has 4 items but the program seems to be counting more items. I was trying to create a function that would count the items that were capitals within the text file.

#include<iostream>
#include<fstream>
using namespace std;
bool ckeckCap(char ch){
    if((ch>='A')&&(ch<='Z')){
        return true;
        return false;
    }
}
int main(){
    ifstream inputFile;
    char ch;
    bool cap;
    int capa=0;
    int count=0;
    inputFile.open("input.txt");
    if(!inputFile.is_open()){
        cout<<"Error opening...Aborting"<<endl;
        return 0;
    }
    while(!inputFile.eof()){
        inputFile>>ch;
        cap=ckeckCap(ch);
        if(cap==true)
        capa=capa+1;
        //inputFile>>ch;
    }
    cout<<capa;
    return 0;
}

Your function `ckeckCap` has undefined behavior if `ch` is not capitalized - what does it return? Your `return false;` should be outside the `if` statement. — JohnFilleau, Feb 25 '20 at 18:20
hey i tried that but it still seems to be giving the same trouble — Amanda Henry, Feb 25 '20 at 18:24
It looks like maybe you meant `return false;` to be outside of the `if`. — François Andrieux, Feb 25 '20 at 18:25
Can you provide an input file and observed output that show the behavior please? — JohnFilleau, Feb 25 '20 at 18:26
Please take a look at [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons). — François Andrieux, Feb 25 '20 at 18:26
the sample data which i used for the input was ABCD the output which i was getting was five which was really confusing for me. when i tried to see the characters which it was outputting the program was outputting two capital d — Amanda Henry, Feb 25 '20 at 18:29
@AmandaHenry that is a common side effect of the bug pointed out by François one comment above. — user4581301, Feb 25 '20 at 18:33
BTW, any statements after a `return` are not executed. So in your `ckeckCap` function, the `return false;` is never executed. — Thomas Matthews, Feb 25 '20 at 22:33
You should use `std::toupper` before comparing the characters. Your program may produce unwanted behavior if the User enters 'e'. — Thomas Matthews, Feb 25 '20 at 22:34

463035818_is_not_a_number · Answer 1 · 2020-02-25T18:57:14.500

You see inconsistent number of entries read from the file because of

while(!inputFile.eof())

This is wrong. See Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?

Further, this:

bool ckeckCap(char ch){
    if((ch>='A')&&(ch<='Z')){
        return true;
        return false;
    }
}

Should be

bool ckeckCap(char ch){
    if((ch>='A')&&(ch<='Z')){
        return true;
    }
    return false;
}

or rather

bool ckeckCap(char ch){
    return ((ch>='A')&&(ch<='Z'));
}

Note how this is shorter, easier to read and you cannot make the same mistake as above.

In your code, the return false; can never be reached. If the condition evaluates to false you do not return from a function declared to return a bool. This invokes undefined behavior.

Not the problem in your code, but this looks suspicious:

char ch;
bool cap;

Using variables before they are initialized is a common mistake (potentially also UB). It can be avoided by declaring variables only when you can initialize them with some meaningful value, or if this is not possible do initialize them with some value, eg:

char ch = ' ';
bool cap = false;

Definitely not least. The while (! EOF) bug is what the asker was tripping over. — user4581301, Feb 25 '20 at 18:54
@user4581301 I didnt read the comments in detail and the question isnt specific on what is the problem. Anyhow, agree and will edit — 463035818_is_not_a_number, Feb 25 '20 at 18:55
No worries. I Had a comment written recommending that the asker include the extra details in the question but either forgot to click Add Comment or was snarky enough to trigger an unkind comment deletion. I'm a bit tired and grumpy today so it's hard to say which is more likely. — user4581301, Feb 25 '20 at 19:00

score 1 · Answer 2 · answered Feb 25 '20 at 19:12

1

Basically all problems have been mentioned already.

And, besides of eliminating the obvious bugs, I will show additionally a more modern C++ solution. By using modern language constructs, errors like the above can be avoided.

Please see:

#include <iostream>
#include <fstream>
#include <algorithm>
#include <iterator>
#include <cctype>

int main() {

    // Open the input file and check, if it could be opened
    if (std::ifstream inputFileStream("r:\\input.txt"); inputFileStream) {

        // Iterate over the characters in the input file and output count of uppercase characters
        std::cout << std::count_if(std::istreambuf_iterator<char>(inputFileStream), {}, std::isupper);
    }
    else {
        // If the input file could not be opened, then inform user
        std::cerr << "\n*** Error: Could not open input file\n";
    }
    return 0;
}

answered Feb 25 '20 at 19:12

Armin Montigny

7,879
3
11
29

tbh I was tempted to downvote, because I dont want SO to be a "gimme the codez" platform, and "gimme the codez" is almost all this answer is doing – 463035818_is_not_a_number Feb 25 '20 at 19:23
@idclev463035818 I upvoted both answers. I think this answer is a nice complement to show an alternative to the DIY version in OP:s post - and it has descriptions for each step. – Ted Lyngmo Feb 25 '20 at 19:57
@TedLyngmo yeah probably this was the wrong answer to deserve a negative comment. It doesnt claim to answer the question but clearly states that is just presents an alternative way. Code is clear and nicely explained. Still i dont like that the problems are labeled as "obvious", but I guess I am too picky once again – 463035818_is_not_a_number Feb 25 '20 at 20:02
@idclev463035818 Nah, it didn't need to have the _obvious_ part in it, I agree. – Ted Lyngmo Feb 25 '20 at 20:08

My character counting code is miscounting characters. Why?

2 Answers2