0

I am using cin.getline() to store user input in a character array, and attempting to parse input to only allow numbers ranging from 1 to 4 to be entered. Things work fine under a specific set of circumstances: correct input is entered on the first try, OR 2 or less characters are entered, and correct input is entered after. An example is below.

[Expected behavior]
Enter input: 1 [ENTER]
Input accepted

[Expected behavior]
Enter input: rw [ENTER]
Incorrect input. Please try again.
Enter input: 1 [ENTER]
Input accepted

[Unexpected behavior]
Enter Input: rtw [ENTER]
Incorrect input. Please try again.
Enter Input: 1 [ENTER]
Incorrect input. Please try again.
Enter input: 1 [ENTER]
Incorrect input. Please try again.
[This will continue indefinitely]

I have tried everything from clearing the input buffer, to resetting the character array to null terminators in an attempt to see if it was still holding values from previous input (like in the unexpected behavior, if a "tw" was somehow still in memory). I think I may have a problem similar to this discussion, but I am not 100% sure. When I attempt to clear the input buffer, it waits for a second set of input, and I am unsure why. When I print the results of inputLength, after an "unexpected behavior" run, it shows that there are still 2 or 3 characters in the array, when I have only entered 1. When removing cin.clear()/cin.ignore(), a second input is not needed, but then the above behavior happens. I appreciate any help.

I have posted relevant code below.

char* ValidateInput() {
    const int maxInput = 25;
    char userInput[maxInput] = { "\0" };
    int inputLength = 0;
    bool correctInputBool = false;

    while (!correctInputBool) {
        // subtract 1 to allow for null terminator at end of array
        cin.getline(userInput, (maxInput - 1), '\n');


        // I have tried both versions of cin.ignore() below, but neither works
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
        //cin.ignore(numeric_limits<streamsize>::max());



        // calculate how many characters user entered
        inputLength = sizeOfCharArray(userInput, maxInput);

        // I do other things here, there isn't a problem with this. For now, assume all 1-character input is acceptable
        if (inputLength == 1) {
            cout << "Correct input." << endl;
            correctInputBool = true;
        }

        if (!correctInputBool) {
            cout << "Sorry, that input is incorrect. Please try again." << endl;
            cout << "Please enter a number between 1 and 4." << endl;
        }
        return userInput;
    }

int sizeOfCharArray(char input[], int maxSize) {
    // all values in input are set to "\0", so count all characters that are not null
    int userSize = 0;
    for (int index = 0; index < maxSize; index++) {
        if (input[index] != '\0') {
            userSize++;
        }
    }
    return userSize;
}

EDIT: I've noticed that when I input more than 3 characters, the next run will always drop the inputLength down one value. Inputting 9 characters goes down to 8 when asking for input again, even if only 1 was input.

  • Do you use `cin >>` in your code as well as `cin.getline()`? Or any other formatted extractions from this stream? – Yksisarvinen Jan 22 '20 at 19:15
  • Also, `*"\0"` triggers my UB senses (though I'm not 100% sure about that). You wanted `'\0'` (a single char) instead. – Yksisarvinen Jan 22 '20 at 19:16
  • @Yksisarvinen , I am only using cin.getline() at the moment. And thank you for the correction, I had `"\0"` in my code, but added the `*` after the compiler yelled at me. I have changed it above as well. –  Jan 22 '20 at 19:18
  • To me I cannot reproduce the unexpected behaviour. – rjhcnf Jan 22 '20 at 19:25
  • @rjhcnf , I copied my project to another editor (codeblocks instead of visual studio), and the problem persisted. When I input more than 3 characters, the next run will always drop the `inputLength` down one value. Inputting 9 characters goes down to 8 when asking for input again, even if only `1` was input. –  Jan 22 '20 at 19:30
  • 1
    There are a few typos in your code, like a missing semicolon after ` cout << "Correct input."` And `ValidateInput` is missing the closing `}`. Maybe it's just a misplaced bracket, because right now the `while (!correctInputBool) {` will always encounter `return userInput;`, so the loop doesn't make sense in this form. – Lukas-T Jan 22 '20 at 19:38
  • Could you tell how are you calling the ValidInput function? Is this called in while loop? – rjhcnf Jan 22 '20 at 19:44
  • Also, is there any reason you can't or won't use `std::getline` and `std::string`? – Lukas-T Jan 22 '20 at 19:46
  • @churill I have figured out my problem, and posted my own answer below. I would love to use both `std::getline` and `std::string`, but am unable to due to this being a homework assignment. I am forced to use C-style strings –  Jan 22 '20 at 19:47

2 Answers2

0

I was able to use visual studio and watch both the inputLength and userInput variables. The inputLength indeed only dropped down 1 value because a null terminator was added to the end of the input. Because of this

userInput = "asdf" // with the reminaing values being '\0'
inputLength = 4

userInput = "2'\0'df" // with the following being held in memory: 2 '\0' df '\0' '\0' . . .
inputLength = 3

When I attempted to print the values of userInput, I only ever saw the 2 due to the null terminator; even though the values were still there, they did not print because the compiler saw '\0', and figured there was nothing after. As a result, when I called sizeOfCharArray, all values were being counted that were not null terminators, which included those from previous input.

My updated code is below.

char* ValidateInput() {
    const int maxInput = 25;
    char userInput[maxInput] = { "\0" };
    int inputLength = 0;
    bool correctInputBool = false;

    while (!correctInputBool) {         
        // updated section
        for (int index = 0; index < maxInput; index++) {
            userInput[index] = '\0';
        }
        // subtract 1 to allow for null terminator at end of array
        cin.getline(userInput, (maxInput - 1), '\n');

        // calculate how many characters user entered
        inputLength = sizeOfCharArray(userInput, maxInput);

        // I do other things here, there isn't a problem with this. For now, assume all 1-character input is acceptable
        if (inputLength == 1) {
            cout << "Correct input."
            correctInputBool = true;
        }

        if (!correctInputBool) {
            cout << "Sorry, that input is incorrect. Please try again." << endl;
            cout << "Please enter a number between 1 and 4." << endl;
        }
        return userInput;
    }

int sizeOfCharArray(char input[], int maxSize) {
    // all values in input are set to "\0", so count all characters that are not null
    int userSize = 0;
    for (int index = 0; index < maxSize; index++) {
        if (input[index] != *"\0") {
            userSize++;
        }
    }
    return userSize;
}
  • This shouldn't work, you set the whole buffer to `\0` right after reading it. The updated section probably should come before `cin.getline(...)`? – Lukas-T Jan 22 '20 at 19:49
  • @churill yes, my mistake. It has been fixed now –  Jan 22 '20 at 19:56
0

There are multiple issues in your code, which you will find sooner or later.

BTW. Your current code doesn't compile. I assume you do have ; after cout << "Correct input." and a } between closing brace of if and return, otherwise your example would never loop even once.

Issue you mention in question

You do not clear userInput (and your sizeOfCharArray is not prepared for that).

Let's go step by step with your code:

  1. userInput[maxInput] = { "\0" }; //userInput contains all null characters
  2. User inputs rwt
  3. userInput contains "rwt\0"
  4. Your code correctly sees it as invalid input and asks for input again
  5. User inputs 1
  6. userInput gets overwtritten by the string user inputted, but it is not cleared beforehand. It now contains 1\0t\0
  7. sizeOfCharArray calculates all non-null characters and return 2.
  8. Your loop continues to ask for input.

You return address of local variable

After ValidateInput, the userInput array is dead. Gone forever. And you return address to a dead array, a memory which can be used by the compiler as it pleases.

Your code is too convoluted

This is often an underestimated issue, but simple code = easy reading = less bugs.

You want an integer, right? So how about reading an integer from input?

int GetInput() {
    int result {};
    while (true) { //infinite loop
        cin >> result;

        if(!cin.good()) {
            cout << "Input wasn't a number, please try again.\n";
            cin.clear(); //clear flags that were raised
            cin.ignore(numeric_limits<streamsize>::max(), '\n'); // skip any input remaining in the stream
        } else if (!InputIsValid(result)) {
            cout << "Input not in 1-4 range, please try again.\n";
        } else {
            return result;
        }
    }
}

bool InputIsValid(int input) {
    return input >= 1 && input <= 4;
}

std::cin will raise itsfail bit if it fails to extract type requested (in this case int) and the variable is zeroed (since C++11). If fail bit is set, good() method will return false (i.e. stream is not in a good state). On the next iteration we clear the flags and any remaining input from the stream.
You can also check for valid range of the integer in the loop (here done as a separate function).

We break out of the loop using return statement. If everything is correct (i.e. stream managed to read correct type of input and the input is in valid range), we return the final value to the user.


Related to the issues with qoutes: Single quotes vs. double quotes in C or C++. Please, for your own mental health, don't add symbols randomly until compiler stops showing errors, it's never a good idea. Compiler is your friend, he will help you spot issues, but only if you help it do that.

Community
  • 1
  • 1
Yksisarvinen
  • 18,008
  • 2
  • 24
  • 52