rabotaet by alexanderskiba · Pull Request #12 · kib-courses/python_developer_hw2

alexanderskiba · 2020-04-26T15:32:40Z

No description provided.

romvano

Странная логика с Patient.current_row_idx. Есть подозрения, что будет работать неправильно.
Много однотипного кода, который стоило бы параметризовать. Мб стоит переделать некоторую логику условий на противоположные для уменьшения размера кода

romvano · 2020-05-01T23:24:39Z

+    # Проверка номера документа
+    def _check_doc_id(self, val, doc_type): # в doc_type будем передавать значение из dict объекта по ключу document_type
+        _valid_len = {
+            'Паспорт' : 10,


константы лучше хранить в переменных

Мне кажется, удобнее будет хранить в словаре, потому что константы семантически связаны и нет смысла записывать каждую в отдельную переменную, так будет сложнее поддерживать

romvano · 2020-05-02T00:02:44Z

+    def __get__(self, obj, objtype):
+        return obj.__dict__[self.name] # вернем значение атрибута по ключу(названию атрибута)
+
+    def __set__(self, obj, val):


в этой функции довольно много повторяющегося кода: лучше как-то параметризовать

Исправлено

romvano · 2020-05-02T00:38:50Z

+    def __init__(self,first_name,last_name, birth_date, phone, document_type, document_id):
+        self._info_logger = logging.getLogger('info_logger')
+
+        if not(first_name and last_name and birth_date


при том, что все аргументы конструктора - позиционные и обязательные, эта проверка не нужна

Здесь идет проверка не на пустоту аргумента, а на пустоту строки, листа, None итд

romvano · 2020-05-02T00:43:57Z

+                    f'{self.birth_date} {self.phone} {self.document_type} {self.document_id}')

+    @classmethod
+    def create(cls, first_name, last_name, birth_date, phone, document_type, document_id):


поскольку никакие атрибуты класса в этом методе не используются, а также нет планируемого такого же поведения наследников класса, то метод стоит сделать статическим

Исправлено

romvano · 2020-05-02T00:55:14Z

+            with open('patient.csv', 'r', newline='') as f:
+                reader = csv.reader(f, delimiter=';')
+                rows = [row for row in reader]  # список списков с полями пациента
+                rows[self._row_idx] = data # элементу списка со всеми пациентами присваются данные конкретного пациента


кажется, этот атрибут всегда будет равен количеству сохраненных в текущей сессии объектов

увидел использование этого атрибута в PatientCollection. предыдущий комментарий - про тот случай, когда Patient при готовом файле создается впервые

Атрибут self._row_idx хранит в себе индекс строки, в которой записан текущий объект пациента
Атрибут Patient.current_row_idx - указатель на новую строку файла, куда будет записан следующий пациент.
Он принадлежит классу, а значит общий для всех объектов.
При этом подразумевается, что при каждом "запуске программы" (пользовательской сессии) если существует файл, то существует объект PatientCollection,
который смотрит файл и устанавливает необходимые атрибуты у класса пациент.
Это необходимо для отражения изменения у объектов в файле, при повторном вызове метода save.
Если не хранить указатель на строку, то после изменения атрибута пациента и повторного вызова save в файле вместо изменения появится новая запись.

romvano · 2020-05-02T00:57:26Z

+
+            with open('patient.csv', "a", newline='') as csv_file:
+                writer = csv.writer(csv_file, delimiter=';')
+                if not Patient.is_header_written:  # Пишем заголовок в файл, он должен быть записан всего один раз, поэтому вот так


этот атрибут будет работать верно только при единственном запуске программы. Кажется, что, если запустить этот код для уже готового файла в первый раз, то что-то пойдет не так

Атрибут Patient.is_header_written - флаг показывающий, что записана шапка таблицы.
Он принадлежит классу, а значит общий для всех объектов.
При этом подразумевается, что при каждом "запуске программы" (пользовательской сессии) если существует файл, то существует объект PatientCollection,
который смотрит файл и устанавливает необходимые атрибуты у класса пациент. Если объекта нет, а существуют только
объекты Patient, то заголовок будет записываться на каждой сессии. Если это неправильное поведение, то как обработать
запись заголовка в сессии без, объекта PatientCollection?

romvano · 2020-05-02T01:01:45Z

+                p = Patient(*row) # Распакуем данные пациента из файла для создания объекта
+                p._saved = True # Теперь объект сохранен
+                p._row_idx = ind # Индекс объекта
+                self.patient_list.append(p) # Добавим пациента в список


если файл огромный, то плохо всё хранить в оперативе

Исправлено

rabotaet

9e145bc

romvano reviewed May 2, 2020

View reviewed changes

fixes

cb6e043

Conversation

alexanderskiba commented Apr 26, 2020

Uh oh!

romvano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants