search

fabuloudy / salon

modelling salon work
0 stars 0 forks source link

Замечания #1

Open alesapin opened 3 years ago

alesapin commented 3 years ago
  1. У питона есть единый Style guide -- PEP8. Обязательно нужно привести код к этому стилю https://www.python.org/dev/peps/pep-0008/. В каждом более менее продвинутом редакторе это можно сделать автоматически. В питоне не нужны скобки около операторов https://github.com/fabuloudy/salon/blob/master/salon/room.py#L16.
  2. Код в глобальном пространстве это плохо https://github.com/fabuloudy/salon/blob/master/salon/main.py#L282-L370. Из такого файла будет невозможно что-либо импортировать. Правильно огородить его if __name__ == "__main__". И вообще код в main.py не очень. Если хочется использовать глобальную переменную model, то пусть она будет наверху, чтобы было понятно как она инициализируется. А если у неё какая-то сложная инициализация, то почему она глобальная, почему её не передавать в функции?
  3. Комментарии в коде используются только чтобы закомментировать какой-то мусор: https://github.com/fabuloudy/salon/blob/master/salon/main.py#L136-L139. Что за класс Master https://github.com/fabuloudy/salon/blob/master/salon/master.py#L2? Что такое oneDay = 480 https://github.com/fabuloudy/salon/blob/master/salon/model.py#L8? А это что https://github.com/fabuloudy/salon/blob/master/salon/room.py#L16?)
  4. Не очень понятно зачем нужны подобные геттеры https://github.com/fabuloudy/salon/blob/master/salon/master.py#L13-L17? Если это приватные переменные, то почему они не начинаются с _? Если публичные, то зачем геттер? Почему метод updateData устанавливает всё в нули https://github.com/fabuloudy/salon/blob/master/salon/master.py#L31-L34?
  5. В питоне есть отличная штука -- namedtuple https://docs.python.org/3/library/collections.html#collections.namedtuple. Позволяет писать вот такие классы в 1 строчку https://github.com/fabuloudy/salon/blob/master/salon/request.py#L2-L20, https://github.com/fabuloudy/salon/blob/master/salon/statistics.py#L1.
  6. А если комнат будет 10 https://github.com/fabuloudy/salon/blob/master/salon/salon.py#L16-L19, то будет 10 строк?
  7. Как же сложно получилось https://github.com/fabuloudy/salon/blob/master/salon/model.py#L72-L81? Опять же, комнат может быть 10 и тогда это совсем превратится в лапшу. Почему бы не хранить номер комнаты в request, а в salon хранить мапу объектов номер -> комната?
fabuloudy commented 3 years ago

@alesapin Спасибо за подробное ревью! Я исправила замечания. Можете посмотреть?