WW #11

[Дмитрий Свиридкин]

make
mkdir obj;
g++ -std=c++11 -Wall -Wextra -Werror -c -Iinclude src/main.cpp -o obj/main.o
make: *** No rule to make target 'include/bin_manip.h', needed by 'obj/employees.o'.  Stop.

Что-то забыли закоммитить

[Дмитрий Свиридкин]

of/ifstream являются наследниками i/ostream, так что отдельно для них не стоит делать перегрузку.
Определиться с форматом можно, проверив флаг std::ios::binary

Использовать имена, начинающиеся с нижнего подчеркивания, может себе позволить только стандартная библиотека и копилятор.

friend std::ifstream& operator>>(std::ifstream& is, Employee* e);

Перегрузка оператора для указателя -- что-то очень неочевидное.

Дефолтный конструктор можно реализовать с помощью инициализаторов полей без написания дополнительного кода.

Методы input/output лучше переименовать в read/write -- для действий обычно используют глаголы.

[lebedev.egor]

В условии лабы сказано, перегружать отдельно ("Разделение текстового/бинарного формата происходит в зависимости от типа потока.")

Я бы с радостью не использовал имена, начинающиеся с нижнего подчеркивания, но они в задании были даны (+ Егор Суворов утверждает, что в классе можно)

(Действительно, забыл закоммитить, увы)

[Дмитрий Свиридкин]

Менять имена приватных полей никто не запрещает.
Вообще, в самом стандарте ничего не говорится про идентификаторы в классах. Сказано, что зарезервированы в глобальном пространстве имен. ​https://stackoverflow.com/a/228797. Но классы это не пространства имен. И, объявив класс в глобальном namespace с полями _x, внутри реализации его методов этим _x можно перекрыть какую-то глобальную ерунду.
А если дальше большая буква -- то вообще везде зарезервировано. С этими безумными тонкостями стандарта лучше не связываться.

---

Комментарии к заданию

В этом задании не очень хорошо:

    То, что вывел operator<<, нельзя сразу считать при помощи operator>>. Нарушается симметрия.
    Разделение текстового/бинарного формата происходит в зависимости от типа потока. Это неочевидно и усложняет автотесты. Нельзя записать бинарно в std::stringstream или записать текст в файл (например, для отладки).

Ох, какой ужас... сделайте хорошо, если сможете. :) Тип потока (бинарный/текстовый) всегда можно чекнуть.

[Дмитрий Свиридкин]

С докоммиченными файлами:

src/employees.cpp: In member function ‘virtual void Developer::get_info(std::istream&)’:
src/employees.cpp:70:5: error: ‘strcpy’ was not declared in this scope
   70 |     strcpy(name_, name.c_str());
      |     ^~~~~~
src/employees.cpp:3:1: note: ‘strcpy’ is defined in header ‘<cstring>’; did you forget to ‘#include <cstring>’?
    2 | #include "bin_manip.h"

Не совсем совпадает формат: SalesManager? должно быть раздельно
После == Total salary: N должна быть пустая строка

где-то перепутаны delete/delete[]

[lebedev.egor]

• Теперь оно должно компилироваться не только у меня на ноуте, но и у всех!
• delete заменил за delete[]
• Добавил пустую строку после Total salary
• Поменял имена у приватных полей
• Исправил перегрузку для указателя на перегрузку для ссылки
• Переименовал методы input/output
• Закинул new в member initializer list
• Возможно, еще что-то...

[lebedev.egor]

(Забыл Type поменять)

[Дмитрий Свиридкин]

в read_c_str должен передаваться указатель на существующий буффер и его размер

В общем случае манипуляторы лучше передавать по константной ссылке - они могут быть некопируемыми.

os << ". Sales Manager\n" Точку с пробелом лучше выводить в коде печати массива. Иначе вы неадекватно печатаете объект вне массива.

Магические константы для типов стоит заменить на enum.
Код, разбирающий типы, протек в main -- он должен быть частью функционала модуля, а не пользовательским кодом.

int32_t size;
is >> read_le_int32(size);

В случае фейла чтения -- будет разное поведение в зависимости от стандарта: либо мусор, либо 0. Явно инициализируйте переменные встроенных типов.

---

7.7/10

[lebedev.egor]

Вроде бы исправил все замечания. Сделал что-то немного странное с std::cin >> make_employee(new_e);

[Дмитрий Свиридкин]

    Employee* new_e;
    std::cin >> make_employee(new_e);
    if (new_e) std::cin >> *new_e;

Так а почему бы в манипуляторе сразу все и не прочитать? Зачем разрывать в два этапа?

read_c_str у вас зачем-то создает новый буффер. Его этого не просили. Весь смысл его в том, что ему дают существующий буффер, куда и надо положить строку. И ограничение на его размер.

---

Ну что ж вы все в последний момент испортили :(

==9131== HEAP SUMMARY:
==9131==     in use at exit: 202 bytes in 2 blocks
==9131==   total heap usage: 24 allocs, 22 frees, 107,714 bytes allocated
==9131== 
==9131== 101 bytes in 1 blocks are definitely lost in loss record 1 of 2
==9131==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==9131==    by 0x10AB21: Employee::Employee() (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10B0A9: SalesManager::SalesManager() (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10B9E7: operator>>(std::basic_ifstream<char, std::char_traits<char> >&, EmployeesArray&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10A67E: ea_load(EmployeesArray&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10A92C: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131== 
==9131== 101 bytes in 1 blocks are definitely lost in loss record 2 of 2
==9131==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==9131==    by 0x10AB21: Employee::Employee() (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10AC9F: Developer::Developer() (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10B9C4: operator>>(std::basic_ifstream<char, std::char_traits<char> >&, EmployeesArray&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10A67E: ea_load(EmployeesArray&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)
==9131==    by 0x10A92C: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/lab_11/check/lab_11/lab_11)

8/10

[lebedev.egor]

Эх, в манипуляторе просто забыл убрать строку 58 с выделением памяти, которое явно осталось с прошлой версии:(