0

I am working on an assignment in C++ meant to teach us more about objects and OOP. Below is my code. The gist of it is, the user enters some input, and the program counts the number of vowels or consonants (chosen by the user), the total number of characters entered, and the total number of end-of-lines.

I am having three issues:

  1. The portion of code I've commented out is causing an infinite loop when left in. It causes the output from the countChars function to be printed infinitely, as well as the output asking the user if they would like to put in more input.
  2. The countChars function isn't counting EOL properly. I think it is most likely due to my unfamiliarity with the EOL. How do I signify it in my conditional statement? If I want to say, "increment if it has a value of '0'", I say, if (variable == 0). How do I tell C++ to increment if something is an EOL?
  3. countChars outputs random, negative values for the counts. I do notice the values changing depending on what I type (except EOL), but I am not sure why I am getting negative values. I'm not sure how to fix it beyond using an unsigned int and initializing the values.

Also, I foresee people telling me to use the getline function, but we had very specific instructions to use cin.get (we're supposed to learn a bit of everything, after all) so please, avoid a fix that uses getline.

Header file:

/*
+----------------------------------------+
|               CountChars               |
+----------------------------------------+
| -countVorC : Integer                   |
| -countEOL : Integer                    |
| -totalChars : Integer                  |
| -vowelCount : Boolean                  |
+----------------------------------------+
| <<constructor>>                        |
|   CountChars()                         |
| +inputChars() :                        |
| +vowelCheck(characterToCheck : Boolean)|
| +setVowelCount(VorC : Character)       |
| +getCountVorC() : Integer              |
| +getCountEOL() : Integer               |
| +getTotalChars() : Integer             |
| +getVowelCount() : Boolean             |
+----------------------------------------+
*/

using namespace std;

#ifndef COUNTCHARS_H
#define COUNTCHARS_H

class CountChars
{
private:
    unsigned int countVorC;
    unsigned int countEOL;
    unsigned int totalChars;
    bool vowelCount;
public:
    CountChars();
    void inputChars();
    bool vowelCheck(char characterToCheck);
    void setVowelCount(char VorC);
    int getCountVorC();
    int getCountEOL();
    int getTotalChars();
    bool getVowelCount();
};

#endif

Implementation file:

#include <iostream>
#include <iomanip>
#include <string>
#include <cctype>
#include <cstdio>
#include "CountChars.h"

using namespace std;

CountChars::CountChars()
{
    unsigned int countVorC = 0;
    unsigned int countEOL = 0;
    unsigned int totalChars = 0;
    bool vowelCount = false;
}

void CountChars::inputChars()
{
    int letter;

    while ((letter = cin.get()) != EOF && letter != EOF){
        if (vowelCount == true && (vowelCheck(letter) == true)) {
            countVorC++;
        }
        else if (vowelCount == false && (vowelCheck(letter) == false)) {
            countVorC++;
        }

        if (isalpha(letter)) {
            totalChars++;
        }

        if (letter == '\n') {
            countEOL++;
        }
    }
}

bool CountChars::vowelCheck(char characterToCheck)
{
    characterToCheck = toupper(characterToCheck);

    if ((isalpha(characterToCheck)) &&
       (characterToCheck == 'A' || characterToCheck == 'E' ||
       characterToCheck == 'I' || characterToCheck == 'O' ||
       characterToCheck == 'U')) {
       return true;
    }
    else {
        return false;
    }
}

void CountChars::setVowelCount(char VorC)
{
    VorC = toupper(VorC);

    if (VorC == 'V') {
        vowelCount = true;
    }
    else {
        vowelCount = false;
    }
}

int CountChars::getCountVorC()
{
    return countVorC;
}

int CountChars::getCountEOL()
{
    return countEOL;
}

int CountChars::getTotalChars()
{
    return totalChars;
}

bool CountChars::getVowelCount()
{
    return vowelCount;
}

Main:

#include <iostream>
#include <iomanip>
#include <string>
#include <cctype>
#include <cstdio>
#include "CountChars.h"

using namespace std;

void printCounts(CountChars);

int main()
{
    char VorC;
    char repeat = 'Y';
    CountChars charCounter;

    cout << "Welcome to the Character Counter Program!" << endl;
    cout << "\nWould you want to count vowels or consonants?" << endl;
    cout << "Type 'V' for vowels and 'C' for consonants: ";
    cin >> VorC;
    cout << endl;

    while (toupper(VorC) != 'V' && toupper(VorC) != 'C') {
        cout << "\nSorry, that was an invalid choice. Please try again: ";
        cin >> VorC;
        cout << endl;
    }


    do {
        cout << "You may being typing input below.\n" << endl;

        charCounter.setVowelCount(VorC);
        charCounter.inputChars();
        cin.clear();
        printCounts(charCounter);

        cout << "\nWould you like to enter new input?" << endl;
        cout << "Type 'Y' for yes or 'N' for no: ";
        cin >> repeat;
        cout << endl;

        while (toupper(repeat) != 'Y' && toupper(repeat) != 'N') {
                cout << "\nSorry, that was an invalid choice. Please try again: ";
                cin >> repeat;
                cout << endl;
        }

    } while (toupper(repeat) == 'Y');

    cout << "\nThank you for using the Character Counter Program!\n" << endl;

    system("pause");
    return 0;
}

void printCounts(CountChars charCounter)
{
cout << "\nTotal characters: " << charCounter.getTotalChars() << endl;

        if (charCounter.getVowelCount() == true) {
            cout << "Total vowels: " << charCounter.getCountVorC() << endl;
        }
        else {
            cout << "Total consonants: " << charCounter.getCountVorC() << endl;
        }

        cout << "Total end-of-lines: " << charCounter.getCountEOL() << endl;
}
stefan
  • 10,215
  • 4
  • 49
  • 90
  • 1
    Why are you casting to `char` when doing `cin >> char(VorC);`? Why aren't you just doing `cin >> VorC;`? – Cornstalks Sep 13 '14 at 04:35
  • 1
    I think you want the OR operator `||` – Jason Sperske Sep 13 '14 at 04:37
  • I've gotten into the habit of casting everything to the type I want because it can very easily cause bugs. At any rate, I don't see why that's important, because it is not causing any of the bugs. I just retried my program with the casting removed. All the bugs are still there, cast or no cast. –  Sep 13 '14 at 04:38
  • @JasonSperske I have a lot of conditional statements in my code. Where are you suggesting I use the OR operator? –  Sep 13 '14 at 04:39
  • With the condition `while not C or not V` – Jason Sperske Sep 13 '14 at 04:40
  • If `cin.get()` returns `EOF`, `cin`'s `eofbit` is set, and when you try to read `repeat` it will simply fail. Thus, `repeat` remains `'Y'` and you get an infinite loop. – T.C. Sep 13 '14 at 04:45
  • @JasonSperske I think you are wrong. If I used an OR operator, it would always evaluate to true and cause an infinite loop (it's always going to not be equal to one of them). I want the loop to execute when it isn't equal to either of them (using an AND). That part of my code is not causing any errors. It is the while loop that comes after it asking for Y or N that is causing problems. –  Sep 13 '14 at 04:45
  • 1
    @AxelFinkel: [casting isn't always a good idea...](http://ideone.com/sVwGsS) – Cornstalks Sep 13 '14 at 04:50
  • I'm sort of surprised that the code with the cast actually compiled. What's the compiler you are using? – T.C. Sep 13 '14 at 05:05
  • @T.C.Visual Studio 2012 –  Sep 13 '14 at 05:07
  • This is one of the places that Visual C++ is horribly horribly broken. Interplay of two bugs. One, cast to same type is not creating a temporary copy (according to the Standard it is required to). Second, non-const reference is binding to an rvalue expression. Luckily later compiler versions will probably fix these. – Ben Voigt Sep 13 '14 at 05:35
  • Compile with all warnings and debug info and **use the debugger** – Basile Starynkevitch Sep 13 '14 at 05:54
  • @axel I see what you mean and I agree, the correct answer has taught me a lot, this has been rather fun to follow :) – Jason Sperske Sep 13 '14 at 15:55

1 Answers1

3

cin.get() returns an int

You have:

char letter;

while ((letter = cin.get()) != EOF)

If the plain char is an unsigned type, as it is on some machines, then this will never evaluate to true because the value -1 (the normal value for EOF) is assigned to an (unsigned) char, it is mapped to 0xFF, and when 0xFF is compared with an int like EOF (that is still -1), the answer is false, so the loop continues.

The fix for this is to use int letter instead of char letter. (Note that there's a different problem with the code as written if char is a signed type; then, the character with code 0xFF — often ÿ, y umlaut, U+00FF, LATIN SMALL LETTER Y WITH DIAERESIS — is misinterpreted as EOF. The fix is the same; use int letter;).

I suspect, though, that this is only part of the trouble.


EOF is not EOL

In the same function, you also have:

    if (letter == EOF) {
        countEOL++;
    }

You know that letter is not EOF (because the loop checked that). Also, you wanted to count EOL, not EOF (there is only one EOF per file, though if you keep attempting read beyond EOF, you will get EOF returned repeatedly). You probably need:

    if (letter == '\n') {
        countEOL++;
    }

or maybe you want to define EOL and compare to that:

    const int EOL = '\n';
    if (letter == EOL) {
        countEOL++;
    }

cin leaves the newline in the input

In the code:

cout << "Type 'V' for vowels and 'C' for consonants: ";
cin >> char(VorC);
cout << endl;

while (toupper(VorC) != 'V' && toupper(VorC) != 'C') {
    cout << "\nSorry, that was an invalid choice. Please try again: ";
    cin >> char(VorC);
    cout << endl;
}

The first cin operation leaves the newline in the input stream. If the use typed 'Y', say, then the next cin operation (inside the loop) will read the newline, and since the newline is neither 'V' nor 'C', it will complain again (but would then wait for more input).

Add #include <limits> and use:

cin.ignore(numeric_limits<streamsize>::max(), '\n');

to read past the newline.

Again, this isn't the whole problem.


Can't continue reading cin after EOF

And the final installment, I think…

Your commented out code is:

/*do {
    cout << "You may being typing input below.\n" << endl;*/

    charCounter.setVowelCount(VorC);
    charCounter.inputChars();

    /*cout << "Would you like to enter new input?";
    cout << "Type 'Y' for yes or 'N' for no: " << endl;
    cin >> char(repeat);
    cout << endl;
        while (toupper(repeat) != 'Y' && toupper(repeat) != 'N') {
            cout << "\nSorry, that was an invalid choice. Please try again: ";
            cin >> char(repeat);
            cout << endl;
        }
} while (toupper(repeat) == 'Y');*/

Note that the call to charCounter.inputChars() doesn't stop until cin has reach EOF. There isn't any more input at that point, so the cin in the loop (that is commented out) will fail every time, never generating a 'Y'. You need to clear the errors on cin so that you can enter more data, such as the answer to the 'more input' question.

I wonder if you've confused EOL and EOF in the reading code. Maybe you intended to read only to the end of a line rather than to the end of the file. Then your loop condition (the one I mentioned first) should be:

int letter;

while ((letter = cin.get()) != EOF && letter != '\n')  // Or EOL if you define EOL as before

You should always be prepared for any of the input operations to return EOF when you didn't really expect it, as here.


Ever the optimist! The previous installment was not the final one.

The constructor doesn't construct

I still have the issue of junk being printed, though. For example, it says Total characters: -85899345.

I tried to compile your code:

$ g++ -O3 -g -std=c++11 -Wall -Wextra -Werror -c CountChars.cpp
CountChars.cpp: In constructor ‘CountChars::CountChars()’:
CountChars.cpp:13:18: error: unused variable ‘countVorC’ [-Werror=unused-variable]
     unsigned int countVorC = 0;
                  ^
CountChars.cpp:14:18: error: unused variable ‘countEOL’ [-Werror=unused-variable]
     unsigned int countEOL = 0;
                  ^
CountChars.cpp:15:18: error: unused variable ‘totalChars’ [-Werror=unused-variable]
     unsigned int totalChars = 0;
                  ^
CountChars.cpp:16:10: error: unused variable ‘vowelCount’ [-Werror=unused-variable]
     bool vowelCount = false;
          ^
cc1plus: all warnings being treated as errors
$

You've declared local variables in your constructor that hide the class members, so your constructor doesn't actually usefully construct. The junk numbers are because you start with junk.

cin >> char(VorC) does not compile everywhere

Similarly, when I tried to compile Main.cpp, I got errors starting:

$ g++ -O3 -g -std=c++11 -Wall -Wextra -Werror -c Main.cpp
Main.cpp: In function ‘int main()’:
Main.cpp:18:9: error: ambiguous overload for ‘operator>>’ (operand types are ‘std::istream {aka std::basic_istream<char>}’ and ‘char’)
     cin >> char(VorC);
         ^
Main.cpp:18:9: note: candidates are:
In file included from /usr/gcc/v4.9.1/include/c++/4.9.1/iostream:40:0,
                 from Main.cpp:1:
/usr/gcc/v4.9.1/include/c++/4.9.1/istream:120:7: note: std::basic_istream<_CharT, _Traits>::__istream_type& std::basic_istream<_CharT, _Traits>::operator>>(std::basic_istream<_CharT, _Traits>::__istream_type& (*)(std::basic_istream<_CharT, _Traits>::__istream_type&)) [with _CharT = char; _Traits = std::char_traits<char>; std::basic_istream<_CharT, _Traits>::__istream_type = std::basic_istream<char>] <near match>
       operator>>(__istream_type& (*__pf)(__istream_type&))
       ^
/usr/gcc/v4.9.1/include/c++/4.9.1/istream:120:7: note:   no known conversion for argument 1 from ‘char’ to ‘std::basic_istream<char>::__istream_type& (*)(std::basic_istream<char>::__istream_type&) {aka std::basic_istream<char>& (*)(std::basic_istream<char>&)}’
/usr/gcc/v4.9.1/include/c++/4.9.1/istream:124:7: note: std::basic_istream<_CharT, _Traits>::__istream_type& std::basic_istream<_CharT, _Traits>::operator>>(std::basic_istream<_CharT, _Traits>::__ios_type& (*)(std::basic_istream<_CharT, _Traits>::__ios_type&)) [with _CharT = char; _Traits = std::char_traits<char>; std::basic_istream<_CharT, _Traits>::__istream_type = std::basic_istream<char>; std::basic_istream<_CharT, _Traits>::__ios_type = std::basic_ios<char>] <near match>
       operator>>(__ios_type& (*__pf)(__ios_type&))
       ^
…
$

The problem here is the:

cin >> char(VorC);

You really don't want the cast there:

cin >> VorC;

You arguably should check that the input works:

if (!(cin >> VorC)) …process EOF or error…

This same problem afflicts the cin >> char(repeat);, of course.

I don't know why it compiled for you; it should not have done so. With that fixed, it sort of works. I ran into the 'newline still in input' so the inputChars() function got zero characters before EOL, etc. Now it is up to you to deal with that.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Does that mean I have to use an unsigned char? When I try that, the inputChars function doesn't work at all. –  Sep 13 '14 at 04:43
  • The fix for counting EOL worked (I had confused EOF and EOL) and it appears end-of-lines are now being properly counted, so thank you for that. I still have the issue of junk being printed, though. For example, it says `Total characters: -858993455` and the infinite loop is still occurring despite changing `letter` to type `int` and inserting `cin.ignore();` after the `cin` operators. Also, I did not misunderstand the assignment. The user is supposed to be able to enter input until they type in ^Z to signify EOF. A new line should not stop the input stream. –  Sep 13 '14 at 05:00
  • I tested my program out some more, and this is what is causing the infinite loop: `while (toupper(repeat) != 'Y' && toupper(repeat) != 'N') { cout << "\nSorry, that was an invalid choice. Please try again: "; cin >> repeat; cout << endl; }` –  Sep 13 '14 at 05:14
  • Turns out, putting in a `cin.clear();` after calling `inputChars()` was all that was needed to fix the infinite loop problem. The program works fine now, except for the last issue of junk values being printed for `totalChars`, `countVorC`, and `countEOL`. –  Sep 13 '14 at 05:26
  • I'm testing the `cin >> repeat` and it is failing to read for me. You may need to test too. It appears that `cin.clear()` does not allow more input after EOF on `cin` — at least, not for me. I'm not immediately sure what the requisite trick is. Incidentally, you need a mechanism to reset the counts to zero after each iteration. The code in the constructor (as fixed) would almost do (you probably don't want to reset the `vowelCount` on each iteration). There should be some abstraction going on too (for the read V/C or Y/N loops) — but that can wait till the rest is working. – Jonathan Leffler Sep 13 '14 at 05:40
  • I've gone ahead and updated all the code in my original question. Can you see if that all works for you now? It is working fine for me. There is no more infinite loop, I can continue entering input and entering `'Y'` for the `while` loop. Everything works, except it is still printing junk values and I can't figure out why. I have initialized the values of the member variables to `0` but I am still getting values like `-858993455`. –  Sep 13 '14 at 05:52
  • Hmm...once `cin` gets an EOF, the stream is closed, so further operations simply don't work. Pretty damn uncivilized if you're used to C where that is not the behaviour. I'm not sure I'm willing to spend the time working out the alternatives. I found [Repeatedly reading to EOF from `stdin`](http://stackoverflow.com/questions/19749982/repeatedly-reading-to-eof-from-stdin) and [Resume reading from `cin` after EOF](http://stackoverflow.com/questions/10147996/resume-reading-from-iostreamcin-after-ctrlz-eof-ignore-doesnt-work) — you can try an SO search on '[c++] reading from cin after eof' too. – Jonathan Leffler Sep 13 '14 at 06:07
  • Note also my comment about resetting the counters on subsequent loops. The alternative — probably better — is to create the `CountChars` object in an appropriate loop scope so that it is recreated on each iteration of the loop. – Jonathan Leffler Sep 13 '14 at 06:09
  • I realized my error. I was re-declaring the member variables in the constructor. I also took care of resetting the counts with each new instance of `CountChars`. –  Sep 13 '14 at 06:12
  • Yes; like I said — your constructor doesn't construct. – Jonathan Leffler Sep 13 '14 at 06:14