Хочу узнать насколько мой код грамотный, и приемлемый ли

Сам код

Программа имитирует движение n-го кол-ва лифтов на n-ом кол-ве этажей. Пользователь добавляет пассажиров на любом этаже, каждый из них потом выбирает куда поедет. В коде всё прокомментировано, надеюсь будет понятно.


Хочу узнать общую оценку кода от опытных программистов. Нормально, или ещё учиться и учиться?


#include <iostream>
#include <cmath>    //для нахождения модуля
#include <windows.h>    //для задержки консоли(sleep())
using namespace std;
int nbfloor;    //кол-во этажей
int nbel;   //кол-во лифтов
class elevator  //сам лифт
{
private:
    int pos = 1;
    char goal[21] = { 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n', 'n'};
    //нажата ли кнопка на этаже и какая, по умолчанию нет
    bool wanted[21] = {false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false };
    //выбран ли этот этаж как цель, по умолчанию нет
    char condition = 's';   //текущее состояние лифта, u - дв. вверх, d -  дв. вниз

public:
    void newgoal(int a, char ch)    //начать движение при новом человеке
    {
        goal[a] = ch;
        if(condition=='s')
            if (a > pos) condition = 'u';
            else condition = 'd';
    }
    void newwanted(int a)   //начать движение при новой выбранной пассажиром цели
    {
        wanted[a] = true;
        if(condition=='s')
            if (a > pos) condition = 'u';
            else condition = 'd';
    }
    void draw(int a)    //нарисовать текущую строчку для этого лифта(есть ли он сам там, или его цели)
    {
        if (goal[a] == 'u')
        {
            cout << "  " << (char)30;   //стрелка вверх
            if (pos == a) cout << (char)219 << (char)219;   //лифт
            else cout << "  ";
            if (wanted[a])
                cout << (char)4;    //значёк, что сюда нужно пассажиру
        }
        if (goal[a] == 'd')
        {
            cout << "  " << (char)31;   //стрелка вниз
            if (pos == a) cout << (char)219 << (char)219;   //лифт
            else cout << "  ";
            if (wanted[a])
                cout << (char)4;    //значёк, что сюда нужно пассажиру
        }
        if (pos == a && goal[a] == 'n')
        {
            if (wanted[a]) cout  << "   " << (char)219 << (char)219<< (char)4;  //лифт и значёк, что сюда нужно пассажиру
            else cout << "   " << (char)219 << (char)219;   //лифт
        }
        else if (goal[a] == 'n')
        {
            if (wanted[a])
                cout << "     "<<(char)4;   //только значёк
            else cout << "     ";
        }
    }
    int getpos(){return pos;}
    char getdir(){return condition;}
    void move();    //прототип движений за "кадр" в программе
};
elevator* el[6];    //массив указателей чтобы можно было создавать новые обьекты в цикле
void draw() //рисует каждую строчку обращаясь к каждому лифту
{
    for (int n = 0; n < nbfloor; n++)   //для каждого этажа
    {
        if(nbfloor-n<10) cout << " "<<nbfloor - n;  //ряд цифр
        else cout << nbfloor - n;
        for (int j = 0; j < nbel; j++)  //функция от каждого лифта
        {
            el[j]->draw(nbfloor - n);
        }
        cout << endl;
    }
}
void addpas()   //добавить пассажира
{
    int fl, nr = 20, elev;
    char ch;
    while (true)    //проверка на "дурака"
    {
        cout << "Write floor: "; cin >> fl;
        cout << "Write desired direction 'u' - up, 'd' - down: "; cin >> ch;
        if (cin.good() && fl <= nbfloor && fl>0 && (ch =='u' || ch == 'd'))     //выходим, если правильный ввод
            break;
        cin.clear();
        system("cls");
        cout << "Values must be integer and char, in the written range!\n";
        cin.ignore(2, '\n');
    }
    for (int n = 0; n < nbel; n++)  //находит лифт, который ближе всех к пассажиру и с подходящим направлением движения
    {
        if (abs(el[n]->getpos() - fl) < nr && (el[n]->getdir() == ch || el[n]->getdir() == 's'))
        {
            nr = abs(el[n]->getpos() - fl); //наименьшее расстояние
            elev = n;   //номер этого лифта
        }
    }
    el[elev]->newgoal(fl, ch);  //команда этому лифту
}
void move() //двигаем каждый лифт
{
    for (int n = 0; n < nbel; n++)
    {
        el[n]->move();
    }
}
void selgoal(elevator* elev)    //выбрать этаж для вошедшего в лифт пассажира
{
    int nb, drfloor;
    for (int n = 0; n < nbel; n++)  //ищем порядковый номер текущего лифта
        if (elev == el[n])
            nb = n;
    cout << "Elevator " << nb+1 << " take a man. Write the desired floor: ";
    while(true)     //проверка на "дурака"
    {
        cin >> drfloor;
        if (cin.good() && drfloor < 11 && drfloor>0)       //выходим, если правильный ввод
            break;
        cin.clear();
        system("cls");
        cout << "Numbers must be integers, in the written range!\n";
        cin.ignore(2, '\n');
    }
    el[nb]->newwanted(drfloor);     //добавляем цель движения лифту
}
void elevator::move()   //передвижения за "кадр"
{
    for (int n = 0; n < nbfloor+1; n++)     //для каждого этажа
    {
        if (condition == 's'&&goal[n] != 'n')   //если лифт стоит, но он где-то вызван, то мы его запускаем
        {
            if (n > pos) condition = 'u';
            else condition = 'd';
        }
    }
    for (int n = 0; n < nbfloor+1; n++)     //если лифт стоит, но пассажиром нажата кнопка ехать, то мы его запускаем
    {
        if (wanted[n]&&goal[n] == 'n'&&!wanted[pos]&&condition=='s')
        {
            if (n > pos) condition = 'u';
            else condition = 'd';
        }
    }
    switch (condition)      //смотрим, что делает лифт
    {
    case 's': if (goal[pos] != 'n')     //если лифт стоит там где заходит пассажир, запускаем функцию
            {
                goal[pos] = 'n';
                selgoal(this);
            }
              if (wanted[pos]) { cout << "Passenger is delivered!"; Sleep(600); wanted[pos] = false; }
              break;
    case 'd': if (goal[pos] != 'n') { goal[pos] = 'n'; condition = 's'; selgoal(this);}
              //если лифт заехал туда, где нужно высадить пассажира, останавливаем лифт
              else pos--;
        if (wanted[pos]) { condition = 's'; }
        break;
    case 'u': if (goal[pos] != 'n') { goal[pos] = 'n'; condition = 's'; selgoal(this);}
              //если лифт заехал туда, где нужно высадить пассажира, останавливаем лифт
              else pos++;
        if (wanted[pos]) { condition = 's'; }
        break;
    }
}


int main()
{
    char ans;
    while (true)    //проверка на "дурака"
    {
        cout << "Write number of floors - >0, <21 : "; cin >> nbfloor;
        cout << "Write number of elevators - >0, <7 : "; cin >> nbel;
        if (cin.good() && nbfloor < 21 && nbfloor>0 && nbel > 0 && nbel < 7)    //выходим, если правильный ввод
            break;
        cin.clear();
        system("cls");
        cout << "Numbers must be integers, in the written range!\n";
        cin.ignore(2, '\n');
    }
    for (int n = 0; n < nbel; n++)      //создаем обьекты
        el[n] = new elevator;
    do {
        system("cls");
        draw(); //рисуем
        cout << "Add new passenger - p"
            << "\n Wait - w"
            << "\n Quit - q     ";
        cin >> ans;
        if (ans == 'p') { addpas(); continue; }
        if (ans == 'w') move();     //двигаем
    } while (ans != 'q');       //выход при 'q'
    return 0;
}

Ответы (1 шт):

Автор решения: HolyBlackCat

Ну поехали...

  • #include <windows.h> //для задержки консоли(sleep()) - непереносимо, в <thread> есть std::this_thread::sleep_for(...);.

  • using namespace std; не рекомендуется использовать, см. Why is "using namespace std;" considered bad practice?.

  • int nbfloor; //кол-во этажей int nbel; //кол-во лифтов - лучше стараться не использовать глобальные переменные без большой необходимости. Можно положить переменные и функции, которые их используют, в еще один класс.

  • Вместо повторения false кучу раз для инициализации массва, лучше использовать цикл или std::fill. Или, если нужно заполнить нулями, до достаточно = {}.

  • char condition = 's'; и другие подобные переменные - вместо них лучше использовать enum, или, если вариантов всего два, можно bool.

  • Стоит добавить проверки, что в функции не передали неправильный номер этажа.

  • На геттеры нужно навесить const.

  • Хорошо, что делаете cin.clear(), но я не уверен, что cin.ignore(2, '\n'); достаточно. Кажется, что вместо 2 должно быть std::numeric_limits<std::streamsize>::max().

  • Магические числа 21 и 6 повторяются несколько раз, их лучше вынести в константы. А еще лучше не иметь максимальное ограничение, а массивы заменить на std::vector.

  • system("cls"); непереносимый.

  • el[n] = new elevator; - в современном C++ лучше не использовать new и delete, только если вы не делаете свой контейнер или умный указатель. Вместо него здесь бы хорошо смотрелся std::unique_ptr и std::make_unique.

→ Ссылка