Opened 4 years ago

Closed 4 years ago

#644 closed ожидается проверка (задача сдана)

WW #11

Reported by: Filippov Denis Owned by: Sokolov Viacheslav
Component: WW cpp_io Version: 3.0
Keywords: Cc:

Description


Change History (6)

comment:1 Changed 4 years ago by Sokolov Viacheslav

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

По условию должно быть ==, а не ====.

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

В реализации le-операций используется порядок байт в системе

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

https://stackoverflow.com/questions/4897844/is-sizeofbool-defined-in-the-c-language-standard

 52   len = std::strlen(str);
 53   assert(str[len] == 0); 

зачем нужен этот assert?

какая мотивация не использовать istream.read по аналогии с ostream.write (кажется, что чтение и запись аналогичны друг другу)?

Кажется, если что-то пошло не так (не удалось чтение / запись), стоит остановить работу приложения

7 #define MAXNAMESIZE 101
в современном c++ для этого есть ключевое слово constexpr

Вместо флажка isExit проще было бы сделать цикл while(true) + break, а еще лучше
while (std::cin && std::cin >> command) + break

comment:2 Changed 4 years ago by Filippov Denis

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

comment:3 Changed 4 years ago by Sokolov Viacheslav

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

9 enum EmployeeTypeEnum?

10 {
11 EMPLOYEE,
12 DEVELOPER,
13 SALESMANAGER
14 };

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

Не все манипуляции проверяются на успешность (например, в main.cpp)

bool(cond)

у примитивных типов нет конструкторов, лучше явно выражать намерение с помощью static_cast<bool> (но можно и ничего не писать вообще, будет эквивалентно)

Для ошибок есть специальный поток std::cerr

std::cout << a << b;
Происходит 2 операции вывода. Первая операция может не получиться, а вторая - получиться. Флаги нужно проверять после каждой операции вывода (что конечно же затруднительно).
Если для записи это не так важно, то вот со чтением все хуже:
in >> _name >> _base_salary >> _has_bonus;
после ввода
aba caba true
флаги будут гласить, что ввод успешно произошел.
По этой причине гораздо проще иметь дело с исключениями.
Если их в арсенале нет, можно использовать манипуляторы, которые возьмут проверку на себя.

372 for (unsigned int i = 1; i <= _employees.size(); i++)
373 out << i << ". " << *_employees[i - 1] << std::endl;
проще же выводить i + 1?

void add(const Employee *e)
такой интерфейс порождает неудобство, требуется копирование. Проще было бы исправить сам интерфейс (принимать объект во владение). В неучебных целях здесь стоило бы использовать std::unique_ptr.

Между командой add и void EmployeesArray::load( const char *fileName ) дублируется код, может быть, можно с этим что-то сделать?

comment:4 Changed 4 years ago by Sokolov Viacheslav

std::cout << a << b;
Происходит 2 операции вывода. Первая операция может не получиться, а вторая - получиться. Флаги нужно проверять после каждой операции вывода (что конечно же затруднительно).
Если для записи это не так важно, то вот со чтением все хуже:
in >> _name >> _base_salary >> _has_bonus;
после ввода
aba caba true
флаги будут гласить, что ввод успешно произошел.
По этой причине гораздо проще иметь дело с исключениями.
Если их в арсенале нет, можно использовать манипуляторы, которые возьмут проверку на себя.

виноват, это неверно. Тем не менее вариант с манипуляторами / исключениями будет лучше, но с другой мотивацией (на паре про это скажу). Если уже что-то поменяли - это хорошо, а не плохо.

comment:5 Changed 4 years ago by Filippov Denis

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

comment:6 Changed 4 years ago by Sokolov Viacheslav

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