Opened 4 years ago

Closed 4 years ago

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

WW_11

Reported by: Jura Khudyakov Owned by: Sokolov Viacheslav
Component: WW cpp_io Version: 2.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

Метод make_copy обычно принято именовать clone

В employees.h стоит пометить классы как final, если от них не предполагается наследоваться

Если произошло исключение, стоит завершить приложение с ненулевым кодом возврата

11 int nbytes = 32 / 8;
В C++ в таких случаях переменную стоит помечать как constexpr (а начиная с С++20 как consteval)

null-terminatin
опечатка?

129 char buf[101];
130 ifs.getline(buf, static_cast<std::streamsize>(101), '\0');
101 стоит вынести в именованную константу, чтобы менять один раз

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

Если хочется что-то свое кинуть, то лучше кидать std::exception. Например, std::logic_error.

достаточно перехватывать const std::exception& (все стандартные исключениия отнаследованы от этого класса)

important to add before input!
почему?

Метод copy_add не используется. Зачем он? Может быть, просто оставить add с семантикой передачи владения?

comment:2 Changed 4 years ago by Jura Khudyakov

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

Исправил
Переименовал
Пометил конкретных работников как final, а от EmployeeArray? в теории можно и отнаследоваться
Не уверен, какой код возврата кидать, так что кину 1
Использовал constexpr
Очепятка, да, исправил
Переделал максимальную длину в статическую константу

Добавил enum class EmployeeID

Кидаю теперь std::logic_error. В main ловлю const std::exception&
Заметил недостаток ловли std::exception-а: если ловить его где-то в коде, а потом снова кидать его же (например, удалить что-то надо), то информация о том, что кинули, теряется, и следующие catch-и ловят просто std::exception, а не то, что кинули изначально.

Пояснил в коде. Были бы тесты - сделал бы для этого тест. Суть - создать объект, считать, а потом передать во владение - плохо, так как при вводе может возникнуть ошибка и программа завершится, не освободив место из-под объекта. Вообще, можно было бы его создать, считать в него и ловить если что exception, но можно обойтись и без этого. А вообще я это место пояснил, так как утечка из-за него изначально была.

copy_add - ну, я хотел, чтобы у использующего была возможность делать push_back с копированием и без, но переделал только на без копирования и оставил пояснение, как надо делать.

Добавил ещё обработку всяких ограничений на входные данные: длина строки кое-где не обрабатывалась, обрабатываю допустимые символы, неотрциательность величин, их максимальное значение, проверяю то, что сумма salary не больше 1e9.
Может показаться странным то, что константы вроде VALID_CHARACTERS и MAX_INT_VALUE не общие, но это довольно логчино: дальше могут быть добавлены сотрудники, у которых разрешены допсимволы, другая длина имени, или используется int64_t и максимальное значение ЗП иное.

comment:3 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed

Если можно наследоваться - то стоит деструктор делать виртуальным либо protected.

Заметил недостаток ловли std::exception-а: если ловить его где-то в коде, а потом снова кидать его же (например, удалить что-то надо), то информация о том, что кинули, теряется, и следующие catch-и ловят просто std::exception, а не то, что кинули изначально.

Наверняка уже давно все понятно, но throw; кинет оригинальное исключение

Note: See TracTickets for help on using tickets.