Хочу узнать насколько мой код грамотный, и приемлемый ли
Программа имитирует движение 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 шт):
Ну поехали...
#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.