Opened 4 years ago

Closed 4 years ago

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

WW #11

Reported by: Gleb Marin Owned by: Sokolov Viacheslav
Component: WW cpp_io Version: 3.0
Keywords: Cc:

Description


Change History (5)

comment:1 Changed 4 years ago by Sokolov Viacheslav

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

Что-то пишется в cerr

На примере из условия пишутся лишние символы:

31 Name: Billy^@

(символ непечатный, поэтому, например, при печати в консоль он может не отображаться)

comment:2 Changed 4 years ago by Gleb Marin

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

comment:3 Changed 4 years ago by Sokolov Viacheslav

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

По поводу CommandLineInterface?, точнее, обработки ошибок. Идейно все хорошо. В c++ по сути не придумали ничего качественно лучше, чем реализованный вариант. У него есть определенные ограничения (в том виде, в котором он реализован), но для нужд лабораторной это неважно. Если Вам интересна тема, рекомендую посмотреть, в каком направлении мысль шла у разработчиков стандартной библиотеки: https://habr.com/ru/post/336012/ (комментарии тоже интересны, например, https://habr.com/ru/post/336012/#comment_10385096). Тема сложная и большая, я в ней сам плохо разбираюсь. С "наскоку" скорее всего будет не очень понятно. Тем не менее это текущее состояние, оно не финальное. Люди работают и обсуждают варианты. Ключевые слова - static exceptions (http://www.open-std.org/jtc1/SC22/wg21/docs/papers/2018/p0709r0.pdf). Обзор https://habr.com/ru/post/430690/. Библиотека https://github.com/TartanLlama/expected .
В общем я вывалил целый ворох информации, она вся скорее всего не будет никак представлена в университете.

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

Для сложных типов вроде std::unordered_map<std::string, std::vector<CommandHandler *>>
стоит делать typedef / using, чтобы было меньше кода

Метод getCopy традиционно именуется clone.

Стоит придерживаться одного стиля именования файлов (snake_case либо CamelCase)

14 uint8_t bytes[4] = {0, 0, 0, 0};
15 is.read(reinterpret_cast<char *>(bytes), 4);

Кажется, здесь используется способ представления (twos complement)

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

is.read(reinterpret_cast<char *>(&cur), 1);
можно же просто is.read() / is.read(cur)?

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

_employees(std::vector<Employee *>(0))

можно просто удалить?

141 if (ind >= this->_employees.size())
142 {
143 return nullptr;
144 }
лучше assert поставить

Между методами
195 std::istream &operator>>(std::istream &is, EmployeesArray? &employees)
241 std::ifstream &operator>>(std::ifstream &ifs, EmployeesArray? &employees)
много дублирования, может быть, можно с этим что-то сделать?

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

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

Метод setHelloString не реализован и не используется

_onRun возможно, лучше именовать _running
activate возможно, лучше именовать run

Не все манипуляции проверяются на успешность

reportDetails можно сделать свободной функцией

Какая мотиваци в функциях Employee::read* использовать *this = ... (конструктор от временных объектов), а не просто сразу считывать поля?

32 is.read(reinterpret_cast<char *>(&cur), 1);
33 _value.push_back(cur);

здесь 0-символ будет добавлен в _value, но std::string и так 0-терминированная

comment:4 Changed 4 years ago by Gleb Marin

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

comment:5 Changed 4 years ago by Sokolov Viacheslav

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

После строчки == Total salary по условию нужен перевод строки

Note: See TracTickets for help on using tickets.