input validation loops?

[jpanther]

Hello everyone,
I have a program that is supposed to read from a text file into two arrays. The user will than input the student ID number to get more information. This input will have a validation check for the following (format XX######):
1- check if the there is less than or more than 8 characters.
2- checks if the first two characters are letters.
3- checks if the last six characters are digits
4- checks if an "X" was entered for exit

Text file looks like:
BC223344 U 10
AB112233 A 134
EE555555 G 18
DE445566 U 122

Problem is:
the program will compile. When I debugged it and entered the first input it recognized the error and displayed the prompt for another input, but when I entered another input it doesn't recognize it. Basically , I just need it to validate the correct format. Any help would be appreciated.

heres my code, I'll just attach the function with the issue, as the program is long.

  //***************************************************************************
void searchID()
{
	//local variable
	int count = 0;
	int cnt;
	string idNum;
	int stuID;
	bool invalidLetter = false;

	cout << endl;
	cout << "Enter ID number of student to find (or X to exit): " << endl;
	cin >> idNum;
	stuID = idNum.length();

	while (idNum != "X" || !invalidLetter)
	{
		// check to see if 'X' was inputted
		if (idNum == "X" || idNum == "x")
		{
			cout << "Goodbye";
			return;
		}

		// checks to see if there is less than and more than 8 characters
		if (stuID < 8 || stuID > 8)
		{
			cout << "ERROR! Invalid entry. ID number is either too short or too long. " << endl;
			cout << "Format as: XX######. Please try again: ";
			cin >> idNum;
		}

		// checks to see if the first two characters are letters
		while (count < 3 && !invalidLetter)
		{
			if (!isalpha(idNum[count]))
			{
				cout << "ERROR! Invalid entry. The first two spaces need to be characters (A-Z)." << endl;
				cout << "Format as: XX######. Please try again: " << endl;
				cin >> idNum;
				count = 0;
			}
			else
				invalidLetter = true;
		}

		// checks to see if the last 6 characters are digits
		for (count = 2; count <= stuID; ++count)
		{
			if (isalpha(idNum[count]))
			{
				cout << "ERROR! Invalid entry. The last six numbers need to be digits (0-9)." << endl;
				cout << "Format as: XX######. Please try again: " << endl;
				cin >> idNum;
				count = 1;
			}
			else
				invalidLetter = true;
		}
		count = 0;		// resets the counter
	} 
	return;
}

[CodeWriter]

Your function is void meaning it returns nothing but you have included a return statement within the function?

When I called your seachID() from the main() it allowed me to enter an id and then froze.

You don't need all those loops I find them confusing you can step through the string with a for loop just once. As you step through idNum if count < 2 then check for isdigit. if count>1 then check for isalpha and as for the length of the string just check for idNum.length()==8

[MiiNiPaa]

Your function is void meaning it returns nothing but you have included a return statement within the function?

Which is perfectly legal and just terminates function.

while (count < 3 You are checking first thre letters here. Namely [0], [1] and [2].

stuID = idNum.length();
//...
count <= stuID;

Last loop iteration will go out of bounds. There is no [8] letter in string containing 8 characters. valid indices are 0 through 7

[CodeWriter]

MiiNiPaa,

Ok a return in void legal. Thank you for that. Any advantage in including return like exiting within a loop?

[MiiNiPaa]

Any advantage in including return like exiting within a loop?

You can terminate function right now instead of making your way through nested control strructures making sure nothing will be executed along the way.

[jpanther]

Thanks CodeWriter and MiiNiPaa for your replies. CodeWriter, you said that I could use one for loop to get my results. I'm a little confused as to how I would accomplish that. I thought that a for loop would be ideal if you have a situation where you have a known amount of loops you want to make, but what if the user continues to make an invalid entry? than it's unknown as to the amount of entries the user will make.

I would like to condense this but I'm not sure how. Could you please explain a little more? Thank you.

[MiiNiPaa]

I actually missed another problem: as soon as invalidLetter is set to true, it stays that way. Nothing changes it back.

Example of eliminating inner loops:

#include <algorithm>
#include <cctype>
#include <string>
#include <iostream>

int main()
{
    bool invalidLetter = true;
    std::string idNum;
    while (invalidLetter) {
        std::cout << "Enter ID number of student to find (or X to exit):\n";
        std::cin >> idNum;
        if (idNum == "X" || idNum == "x") {
            std::cout << "Goodbye";
            return 0;
        }
        if (idNum.size() != 8) {
            std::cout << "ERROR! Invalid entry. ID number is either too short or too long.\n"
                         "Format as: XX######. Please try again\n";
            continue;
        }
        if(not std::all_of(idNum.begin(), idNum.begin() + 2, ::isalpha)) {
            std::cout << "ERROR! Invalid entry. The first two spaces need to be characters (A-Z).\n"
                         "Format as: XX######. Please try again\n";
            continue;
        }
        if(not std::all_of(idNum.begin() + 2, idNum.end(), ::isdigit)) {
            std::cout << "ERROR! Invalid entry. The last six numbers need to be digits (0-9).\n"
                         "Format as: XX######. Please try again\n";
            continue;
        }
        invalidLetter = false;
    }
    std::cout << "Success, your ID " << idNum << " was accepted.\n";
}

[jpanther]

This looks much cleaner and easier to follow. I just have one question, I am not too familiar with this line:

if(not std::all_of(idNum.begin(), idNum.begin() + 2, ::isalpha))

Could you please explain this? Thank you :)

[MiiNiPaa]

THis is algorithm from standard library: http://en.cppreference.com/w/cpp/algorithm/all_any_none_of

It says: iterate over sequence defined by first two iterators and return if all elements are returning true when passed to predicate (isalpha). Then I negate it so conditional statement body will be executed only if not all elements are letters.

[jpanther]

Thank you for that explanation and the web site. Much appreciated. Have a great day!

[CodeWriter]

By single loop I meant one loop through the string idNum. You do of course need an outer loop to keep asking for a valid id number.
Keep in mind I am fairly new to c++ but have programmed a lot in BASIC which is a much easier language to use.

#include <iostream>

using namespace std;

bool testValidity(string id)
{
    bool invalidLetter = false;
    for (unsigned int i=0;i<id.length()-1;i++)
    {
        if (i>1 && isalpha(id[i]))
            invalidLetter = true;
        if (i<2 && isdigit(id[i]))
            invalidLetter = true;
    }
    if (id.length()!= 8)
    invalidLetter = true;
    return invalidLetter;
}

int main()
{
    string idNum = " ";
    cout << "Enter ID number or enter x to end: " << endl;
    cin  >> idNum;
    while (idNum != "x")
    {

        if (testValidity(idNum)==true)
        {
            cout << "INVALID ID" << endl;
            cout << "Must start with two characters A to Z and end with six digits" << endl;
        }
        else
        {
            cout << "VALID ID" << endl;
        }
	    cout << "Enter ID number or enter x to end: " << endl;
	    cin  >> idNum;
    }

	return 0;
}

[jpanther]

Thank you for clarifying that. I like the way you separated the validity test from the user input. Much appreciated. Have a great day, CodeWriter!