Hw2#17
Conversation
romvano
left a comment
romvano
left a comment
There was a problem hiding this comment.
Я оставил кучу комментариев насчет TypeError, но потом увидел, где это обрабатывается. Основные замечания - перенести обработку этой ошибки к остальным, чтобы сразу таких вопросов не возникало, а еще переписать дескриптор через проперти, ибо он почти полностью повторяет логику проперти.
|
|
||
| @logger_decorator_maker() | ||
| def name_check(name): | ||
| if not name.isalpha(): |
There was a problem hiding this comment.
если name не str, то залогируется AttributeError, а не TypeError
| raise ValueError("Incorrect date length") | ||
| born = born[:4] + '-' + born[5:7] + '-' + born[8:] | ||
| for k, i in enumerate(born): | ||
| if i.isdigit() and (0 <= k <= 3 or k == 5 or k == 6 or k == 8 or k == 9): |
There was a problem hiding this comment.
лучше регуляркой, чтобы было понятней
|
|
||
| @logger_decorator_maker() | ||
| def birth_check(born): | ||
| if len(born) != 10: |
There was a problem hiding this comment.
тут тоже может вылезти AttributeError
| continue | ||
| else: | ||
| raise ValueError("Date contains invalid characters") | ||
| if str(datetime.date.today()) >= born: #колхозное сравнение) |
|
|
||
| @logger_decorator_maker() | ||
| def phone_check(phone): | ||
| phone = phone.replace('+', '') |
There was a problem hiding this comment.
то же самое с AttributeError
|
|
||
| @logger_decorator_maker() | ||
| def __set__(self, obj, val): | ||
| if not isinstance(val,str): |
There was a problem hiding this comment.
ага, всё-таки есть проверка на str. Лучше ее переместить ко всем остальным проверкам, чтобы логика была более консистентной
| self.name = name | ||
|
|
||
| def __get__(self, obj, objtype): | ||
| return self.data_get(obj, self.name) |
There was a problem hiding this comment.
по умолчанию self.data_get == None, и любая попытка достучаться до дескриптора будет валиться с ошибкой о том, что data_get not callable. Это точно то поведение, которое планируется по умолчанию?
| def __init__(self, *args, **kwargs): | ||
| pass | ||
| def get_field(self, key): | ||
| if key == 'first_name': |
There was a problem hiding this comment.
более универсально return getattr(self, "_" + key)
| def create(*args, **kwargs): | ||
| raise NotImplementedError() | ||
| first_name = DataAccess('first_name', get_field, set_field, del_field, name_check) | ||
| last_name = DataAccess('last_name', get_field, set_field, del_field, name_check) |
There was a problem hiding this comment.
get_field, set_field, del_field - везде одинаковые. Нужно ли их передавать в параметрах?
Я бы сделал отдельный сеттер для first_name и last_name, чтобы в этом был смысл. Как раз и количество кода в сеттере уменьшится
| def __iter__(self): | ||
| try: | ||
| for i in range(0, len(pnd.read_csv(self.path, sep='|', header=None, dtype=str).index)): | ||
| yield Patient(*pnd.read_csv(self.path, sep='|', header=None, dtype=str).iloc[i]) |
There was a problem hiding this comment.
минус такого подхода в том, что файл каждый раз считывается полностью. А он может быть очень большим, даже больше оперативы
2 participants
No description provided.