Opened 4 years ago

Closed 4 years ago

#680 closed ожидаются исправления (задача сдана)

WW #11

Reported by: Alexander Morozov Owned by: Sokolov Viacheslav
Component: WW cpp_io Version: 1.0
Keywords: Cc:

Description


Change History (3)

comment:1 Changed 4 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

Несоответствия условию:
Sales Manager vs SalesManager?
Base Salary vs Base salary

Классы стоит помечать как final, если от них не предполагается наследоваться, а конструкторы делать explicit (нужно ли пояснить, почему?)

В заголовочных файлах лучше не иметь реализаций методов, потому что при изменениях этих реализаций будут пересобираться все зависимые файлы, хотя в этом может и не быть потребности.

Лучше избегать совпадения имен аргументов и полей класса, чтобы случайно не ошибиться; конструкции вида x(x) вызывают вопросы (нет ли здесь ошибки). Удобно с помощью code style гарантировать различное именование (например, добавляя уникальный префикс к полям классов)

Текущие манипуляторы стоит переименовать, чтобы было понятно, что это манипуляторы. Например, положить их в пространство имен или просто дополнить имя.

В текущей реализации манипулятора bin_int32 важно, что число положительное. Стоит поставить на это assert.

Рекомендую посмотреть на ​https://en.cppreference.com/w/cpp/io/basic_istream/getline

56 ostream &operator<<(ostream &stream, const bin_str &str) {
можно разом вывести все strlen(str.val) + 1 символов

Если манипуляция не удалась, стоит выставить failbit у потока, чтобы больше ничего не происходило.

22 int Employee::get_employee_type() const {
23 if (typeid(*this) == typeid(Developer)) {
24 return 1;
25 } else {
26 assert(typeid(*this) == typeid(SalesManager?));
27 return 2;
28 }
29 }

такая реализация означает, что базовый класс знает обо всех своих предках, что не есть хорошо. При расширении приходится модифицировать базовый класс. Вероятно, чуть лучше было бы сделать виртуальный метод. typeid достаточно дорогая операция, и если что-то можно реализовать без динанимеской проверки типов, лучше так и делать.

Между read_employee и load_bin_employee дублируется код, может быть, можно с этим что-то сделать?

Сейчас в функциях явно прописаны значения 1,2, из-за чего страдает расширяемость - при добавлении новых Employee будет тяжело следить за тем, что везде все правильно поддержано. Стоит использовать enum (class), чтобы держать все возможные значения в одном месте.

Имя это часть базового класса, может, лучше и считывать его в базовом классе?

Функции print частично дублируются, может быть, можно с этим что-то сделать?

comment:2 Changed 4 years ago by Sokolov Viacheslav

Если произошло исключение, стоит в main его перехватить, залогировать сообщение об ошибке и завершить программу с ненулевым кодом возврата

comment:3 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed
Note: See TracTickets for help on using tickets.