WW #11

[Sokolov Viacheslav]

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

Сделать класс, который инкапсулирует всю логику чтения / записи в нужном формате - идея неплохая, но C++ идет скорее по другому пути развития, предлагая все же делать независимые классы. Однако заставлять программиста писать больше кода неразумно, поэтому для подобных целей существуют шаблоны классов / функций (class template / function template).

В текущей реализации есть ряд проблем.
1) (minor) не самое удачное название. Кмк, лучше было бы назвать WriterBinary? / ReaderBinary?.
2) BinaryWritable? использует больше памяти, чем ему необходимо. Более того, логически нужно лишь одно из полей, остальные появляются по техническим причинам. Чтобы решить эту проблему, можно использовать std::variant (так стоило бы делать в реальном проекте), но он нам определенно недоступен. Есть доступная альтернатива: union ​https://en.cppreference.com/w/cpp/language/union . Проблема в том, что union не знает, какого типа данные в нем сейчас содержатся, поэтому эту информацию нужно хранить отдельно. Аналогично с BinaryReadable?.
3) В BinaryReadable? хранятся ссылки, поэтому потребовался такой странный конструктор и вспомогательные функции. Вместо этого стоит хранить указатели, которую как раз таки могут быть null. (и могут быть членами union)
4) Вспомогательные функции write_le_int32 и подобные просто дублируют конструктор, их лучше убрать.
5) Имена вспомогательный функций write_le_int32 и подобных сбивают с толку, потому что не делают того, как названы (никакой записи при вызове не происходит)
6) Название getChar неверное, потому что возвращается не символ, а строка.
7) Часть функций реализована в заголовочном файле (они стали inline, хоть это может и не быть очевидно из декларации) - а зачем?
8) Какая мотивация делать write_le_int32_s и подобные свободными функциями?

Предполагается ли, что write_le_int32_s работает с отрицательными значениями?

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

Вместо typedef bool (*com)(EmployeesArray &ea, std::stringstream &is)
можно использовать более современную запись
using com = bool(*)(EmployeesArray &ea, std::stringstream &is);
Название com не очень говорящее

std::cin.clear();
зачем эта строчка?

Проверка потоков на неуспешность манипуляции во многих местах отсутствует (в манипуляторах, например, где она в первую очередь и нужна)

Если у Employee всегда должно быть имя, то лучше поставить assert в конструкторе

dynamic_cast лучше не использовать, потому что он переносит проверку из compile time в runtime. Хотите избежать дублирования кода - используйте шаблоны (их мы тоже не проходили) или подумайте, как сделать то же с помощью виртуальных функций (это возможно).

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

[Sokolov Viacheslav]

34 RETURN_FAIL_STRM(os, <<, "Base salary: ");

в условии просят Salary

232 RETURN_FAIL_STRM(os, <<, "== Total salary: ");
233 RETURN_FAIL_STRM(os, <<, ea.total_salary());
а здесь нужен еще один перевод строки

41 char name[100];
этого хватит только чтобы вместить 99 символов, поскольку строка 0-терминированная и этот 0 тоже хранится.

10 if (!name)
11 return;
12 setName(name);
13 assert(name != nullptr);

assert и return взаимоисключающие.