Taknie
Taknie

Reputation: 109

Is that kind of controling the while() loop a good practice? C++

I am working on a small database project. I've already made an interface with switch() function, which is supposed to be calling functions and looping till I chose the "EXIT" option. I am controlling the loop by setting specified value into int Loop variable. Is it a good practice to deal with this kind of looping that way? If no, why then?. In other functions, when I have multiple conditions, I use that kind of variables even twice. Maybe should I do it differently?. Does it make sense if I use try(), throw(), catch() exceptions in this case, how would it looks like then? Here's my piece of code:

void mainMenu() {

    vector<Employee> firmEmployees;
    vector<Intern> firmInterns;

    int Loop = 0;
    while (Loop == 0) {

        cout << endl << endl;
        cout << "================ EMPLOYEE DATABASE ================" << endl << endl;
        cout << " HIRE NEW EMPLOYEE (1)" << endl;
        cout << " MANAGE EMPLOYEES  (2)" << endl;
        cout << " HIRE NEW INTERN   (3)" << endl;
        cout << " MANAGE INTERNS    (4)" << endl;
        cout << " EXIT              (5)" << endl;
        cout << " Choose option... ";
        int option;
        cin >> option;

        if (option < 1 || option > 5 || !cin) {
            cout << endl << "---Wrong input!---";
            clearInput(); // cleaning cin
        } else {

            switch (option) {
                default:
                    break;
                case 1:
                    hireEmployee(firmEmployees);
                    break;
                case 2:
                    employeeMenu(firmEmployees);
                    break;
                case 3:
                    hireIntern(firmInterns);
                    break;
                case 4:
                    internMenu(firmInterns);
                    break;
                case 5:
                    Loop = 1;
                    break;
            }
        }
    }
}

EDIT: Another example, more variables.

void fireEmployee(vector<Employee>& sourceEmployee) {

    int repeat = 0;

    while (repeat == 0) {

        cout << "Enter ID to fire an employee: ";
        int id;
        cin >> id;

        if (cin.fail()) {
            clearInput();
            cout << "ID number needed!" << endl;
        } else {

            int buf = 0;

            for (auto &i : sourceEmployee) {

                if (i.getID() == id) {
                    i.Fire();
                    i.setSalary(0);
                    cout << i.getName() << " " << i.getSurname() << " (" << i.getID() << ") has been fired" << endl;
                    buf = 1;
                    repeat = 1;
                }
            }
            if (buf == 0) {
                cout << "No employee with ID: " << id << endl;

            }
        }
    }
}

Upvotes: 2

Views: 293

Answers (1)

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136485

The best practice would be to extract that while loop into a function and do return instead of Loop = 1;. That would be explicit flow control which is easy to read and maintain. E.g.:

void mainMenuLoop(vector<Employee>& firmEmployees, vector<Intern>& firmInterns) {
    for(;;) {
        cout << "\n\n================ EMPLOYEE DATABASE ================\n\n";
        cout << " HIRE NEW EMPLOYEE (1)\n";
        cout << " MANAGE EMPLOYEES  (2)\n";
        cout << " HIRE NEW INTERN   (3)\n";
        cout << " MANAGE INTERNS    (4)\n";
        cout << " EXIT              (5)\n";
        cout << " Choose option... " << flush; // Must flush here.
        int option = -1; // Assign a wrong initial option.
        cin >> option;
        switch (option) {
            case 1:
                hireEmployee(firmEmployees);
                break;
            case 2:
                employeeMenu(firmEmployees);
                break;
            case 3:
                hireIntern(firmInterns);
                break;
            case 4:
                internMenu(firmInterns);
                break;
            case 5:
                return;
            default:
                cout << "\n---Wrong input!---" << endl;
                clearInput(); // cleaning cin
                break;
        }
    }
}

void mainMenu() {
    vector<Employee> firmEmployees;
    vector<Intern> firmInterns;
    mainMenuLoop(firmEmployees, firmInterns);
}

Also note, that in

int option;
cin >> option;

if cin >> option fails option retains its original indeterminate value, which can be one of the available options. It is safer to assign it an initial value which is not a valid option, like -1.

Upvotes: 8

Related Questions