aworks, tests done, but may be clearly code#14
Conversation
romvano
left a comment
romvano
left a comment
There was a problem hiding this comment.
Основные огрехи:
- Слишком много приватных полей с двумя подчеркиваниями. Почти все они могут спокойно быть protected
- Логика проверки значений - оооооооооооооооооооооооооооооооооочень сложная. Прям невероятно. Код можно упростить раз в 5.
Но зато сама чистота кода - норм. За аннотации - плюс в карму
| from homework import assistive | ||
|
|
||
|
|
||
| class PatientFieldsValueError(ValueError): |
There was a problem hiding this comment.
Type, Value и Attribute errors достаточно неплохо отражают суть ошибок, здесь не нужно создавать что-то специфическое. Но за реализацию, конечно, плюсик в карму
| return set_wrapper | ||
|
|
||
|
|
||
| def get_decorator(self_value_to_get_name: str): |
There was a problem hiding this comment.
это всё-таки не декоратор, а фабрика вызываемых объектов
| logger.error(message + ":" + str(args)) | ||
|
|
||
|
|
||
| def set_decorator(set_funct, self_value_to_set_name: str, logger_name: str, *set_funct_args): |
| __PHONE_SIGNIFICANT_DIGITS_NUMBER = 10 | ||
| __UNIVERAL_INNER_PHONE_CODE = "8" | ||
| __RUSSIAN_PHONE_CODE = "+7" | ||
| __ALSO_RUSSIAN_PHONE_CODE = "7" |
There was a problem hiding this comment.
константы в таком виде лучше выносить в отдельный файл, чтобы в случае чего их можно было поменять быстро и без долгих поисков
There was a problem hiding this comment.
плюс, поскольку у нас не планируется какого-то сложного наследования, можно их записать с одним подчеркиванием
| # __patient_logging = PatientLoggingDescriptor() | ||
|
|
||
| @classmethod | ||
| def create(cls, first_name=None, last_name=None, birth_date=None, \ |
There was a problem hiding this comment.
поскольку нет сложной логики наследования и не используются атрибуты класса, метод лучше сделать статическим
| # logging.info("File %s exists" % path_to_file) | ||
| else: | ||
| # logging.info("File %s not exists" % path_to_file) | ||
| raise Exception("File not exists") |
| if ".csv" not in path_to_file: | ||
| # logging.info("File %s type error" % path_to_file) |
There was a problem hiding this comment.
.csv необязательно должно быть в названии файла
| def __inner_dict_filler(self): | ||
| patient_data_frame = pandas.read_csv(self.myfile) | ||
| for index, row in patient_data_frame.iterrows(): | ||
| self.__inner_patient_list.append(Patient(first_name=row["first name"], last_name=row["last name"], \ |
There was a problem hiding this comment.
есть ли смысл хранить всех пациентов в оперативе? Их может быть очень много
| for line in file: | ||
| if count > 0: | ||
| patient = self.patient_creation_from_bytestr(line, encoding="utf-8") | ||
| # нулевая линия - заголовок |
There was a problem hiding this comment.
можно ее сразу пропустить при открытии файла, тогда не придется возиться с count
| def limit(self, n): | ||
| raise NotImplementedError() | ||
| def __contains__(self, item: Patient): | ||
| if type(item) is Patient: |
2 participants
Not so clear, but aworks