Help with Assignment

[Jolom]

This assignment requires me to verify an ID code. Basically I am taking in a user string input and converting it into a char array. After, I have to compare each letter or number using a nested switch and see if it follows a specific sequence. I'm trying to validate each sequence, and if one part doesn't match, then I print out that the some part of the entry is wrong and I list out the appropriate choices for that part.

Ex. User enters AK101
Input is invalid, the second letter must be B, C, or D.

The number validation works, but when I try to validate and print out a validation statement for the second letter, it doesn't work. Can someone explain why?

#include <iostream>
#include <conio.h>
#include <cstdlib>

using namespace std;

int main()
{

    string ID;
    char again = 'y';
    bool firstValid, secondValid, numValid;
    char empID[4];
    char charID;

    do {

    system ("cls");

    firstValid = false; // reset flag to false
    numValid = false;
    secondValid = false;

    cout << "\t\tValid Customer ID Rules:\n\n";
    cout << "1 - A, K, S - Followed by B, C, D, Followed by 101 to 110" << endl;
    cout << "2 - B, H, N - Followed by A, F, G, H, Followed by 113 to 118 or 213 or 220 or 560 or 890" << endl; // goes off screen
    cout << "3 - C or T - Followed by K, L, Z, Followed by 134, 138, 145, or 246" << endl;
    cout << "4 - M, O, Z - Followed by A, D , F, Followed by 177 to 181 or 333 to 335\n\n";

    cout << "Enter a Customer ID: ";
    cin >> ID;

    while (ID.length() != 5) // ID length validation
    {
        cout << "Invalid ID code length. Reenter valid Customer ID: ";
        cin >> ID;
    }

    for (int i = 0; i < ID.length(); i++)
    {
        empID[i] = ID.at(i);
    }

    char *p = &empID[2]; // set pointer to point at 2nd element to view first occurrence of an int

    int numID = atoi(p);
    charID = toupper(empID[0]);

    switch (charID)
    {
        case 'A':
        case 'K':
        case 'S':
            firstValid = true;
            charID = toupper(empID[1]);
            switch (charID)
            {
                case 'B':
                case 'C':
                case 'D':
                    secondValid = true;
                        switch (numID)
                        {
                            case 101 ... 110:
                            numValid = true;
                            break;
                        }
                    if (numValid == false)
                    {
                       cout << "Invalid Digits (Must be between 101 to 110)" << endl; // prints
                    }
                    break;
            }
            break;
            if (secondValid == false)
                {
                    cout << "Invalid sequence. First letter must be followed by B, C, or D"; // doesnt print

                }


        default:
            firstValid = false;
             if (firstValid == false)
            {
                cout << "Invalid start. First Letter must be A, K, B, H, N, C, T, M, O, or Z"; 
            }


    }

    if (firstValid == true && secondValid == true && numValid == true)
    {
        cout << ID << "is valid" << endl;
    }

    else
    {

        cout << ID << "is not valid" << endl;
    }

     cout << "Validate another ID (Y or N): ";
     cin >> again;
     again = tolower(again);
     cin.ignore();
    } while (again == 'y');

}

[salem c]

Well trying to fit "AK101" in char empID[4]; is surely going to fail in mysterious ways.
"AK101" needs a minimum of 6 characters.

> I have to compare each letter or number using a nested switch
If you're going to use such poor practice, then at least make sure your code is indented properly.
https://en.wikipedia.org/wiki/Indentation_style

[Jolom]

I'm just curious what do you mean by poor practice. I'm trying to catch onto all these little details as a programmer. If you mean't not using functions to modularize, then its because the assignment didn't say to use functions, so I avoided it. If it's about the indentation, then it's only cause I've been writing and deleting so I haven't looked over how it's indented to see if it looks nice. Regardless, I managed to debug, so thank you.

[salem c]

Well nested case statements have a way of growing very long, very quickly.

If you have 5 outer cases, each with 5 inner cases, that's 30 separate cases to deal with.

case value:
{
    break;
}

The function is now way over 100 lines, and that's without any functionality.
So yes, modularisation with some functions would be a big help.

> then it's only cause I've been writing and deleting so I haven't looked over how it's indented to see if it looks nice.
Nice to know you hold your audience in such high esteem.

It helps you, because it's a lot easier to see what's going on.

It helps us, because in the 0.1 seconds it takes to see what the general layout of your code is, many will make a decision to either keep reading (if it looks nice) or find something else to do (it looks like dog food).

Many IDE's have an auto-indent feature. A couple of clicks, and your code looks nice once more.
In a competitive place for attention (like a forum), making the best presentation possible certainly can't be harmful to your chances of getting an answer.

Good job on finding the answer BTW :)

[dutch]

This isn't perfect, but I like the idea of putting the info into a data structure. I would have encoded the numbers differently if there were longer runs of consecutive values (perhaps as std::pair's).

#include <iostream>
#include <iomanip>
#include <sstream>
#include <string>
#include <vector>
#include <algorithm> // find()

using std::string;
using std::vector;

struct IDType {
    string firstChar;
    string secondChar;
    vector<int> number;
};

vector<IDType> IdTypes {
    {"AKS", "BCD",  {101,102,103,104,105,106,107,108,109,110}},
    {"BHN", "AFGH", {113,114,115,116,213,220,560,890}},
    {"CT",  "KLZ",  {134,138,145,245}},
    {"MOZ", "ADF",  {177,178,179,180,181,333,334,335}}
};
const int NumTypes = IdTypes.size();

bool checkID(const string& id) {
    std::istringstream iss(id);
    char firstChar, secondChar;
    int number;
    if (!(iss >> firstChar >> secondChar >> number))
        return false;
    int type = 0;
    for ( ; type < NumTypes; ++type)
        if (IdTypes[type].firstChar.find(firstChar) != std::string::npos)
            break;
    if (type == NumTypes)
        return false;
    const auto& t = IdTypes[type];
    return t.secondChar.find(secondChar) != std::string::npos &&
           std::find(t.number.begin(), t.number.end(), number) != t.number.end();
}

string numberString(const vector<int>& nums) {
    if (nums.size() == 0)
        return "";
    std::ostringstream oss;
    oss << nums[0];
    int in_a_row = 1;
    for (size_t i = 1; i <= nums.size(); ++i) {
        if (i < nums.size() && nums[i] == nums[i - 1] + 1)
            ++in_a_row;
        else {
            if (in_a_row > 1)
                oss << (in_a_row > 2 ? '-' : ',') << nums[i - 1];
            if (i < nums.size())
                oss << ',' << nums[i];
            in_a_row = 1;
        }
    }
    return oss.str();
}

void showTypes() {
    std::cout << "\nID Types:\n";
    for (int i = 0; i < NumTypes; ++i)
        std::cout << "  " << i + 1
            << ". [" << std::left << std::setw(4) << IdTypes[i].firstChar
            << "] [" << std::setw(4) << IdTypes[i].secondChar
            << "] {" << numberString(IdTypes[i].number) << "}\n" << std::right;
}

int main() { 
    for (;;) {
        showTypes();

        string id;
        std::cout << "\nEnter ID (or nothing to quit): ";
        std::getline(std::cin, id);
        if (id.size() == 0)
            break;

        std::cout << '\n' << id << " is "
                  << (checkID(id) ? "" : "not ")
                  << "valid.\n";

        std::cout << "Press 'Enter' to continue... ";
        std::cin.ignore(9999, '\n');
    }
}

ID Types:
  1. [AKS ] [BCD ] {101-110}
  2. [BHN ] [AFGH] {113-116,213,220,560,890}
  3. [CT  ] [KLZ ] {134,138,145,245}
  4. [MOZ ] [ADF ] {177-181,333-335}

Enter ID (or nothing to quit): TZ245

TZ245 is valid.
Press 'Enter' to continue... 

[doug4]

Just some additional thoughts (based on the code shown in the initial post):

Line 70 really belongs in a default case of the inner switch statement (line 62).

The inner switch statement might be better served as an if statement. That's a design decision that I would have to evaluate some more.

Line 77 really belongs in a default case of the middle switch statement (line 56).

Line 86 is the only statement needed in the default case of the outer switch statement (line 49).

If you judiciously use default cases, you no longer need 3 different "valid" variables. A single idValid variable can be initialized to false and set true in the inner switch (or initialized to true and set to false in each of the default cases).

If you mean't not using functions to modularize, then its because the assignment didn't say to use functions, so I avoided it.

Wow. That is short-sighted. All programs can theoretically be written without function calls, but functions HELP programmers. Did the assignment say to use white space and indentation? Probably not, but you didn't avoid using them because they HELP programmers (when used properly).

Maybe your teacher hasn't covered writing functions yet--that's a valid reason not to use them. But that's not the reason you gave. Don't dismiss useful tools of the language just because the assignment didn't say to use them.