WW #11 Golovin Valery

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

Оно, конечно, работает и даже форматы правильные. Но

В операторах для манипуляторов лучше везде передавать по const&

write_le_int32(const int32_t& x);
Примитивные типы по константным ссылкам лучше не передавать -- это дополнительные накладные расходы. Передавайте по копии.

   istr >> sales_manager;
   list.add(&sales_manager);

Создавать объект, читать, потом копировать, потом удалять. Может лучше Read, возвращающий указатель, который сразу отдается в массив?

И самое главное:
asan, ubsan, valgrind пора стать вашими лучшими друзьями.

==15750== Memcheck, a memory error detector
==15750== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15750== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==15750== Command: lab_11/lab_11
==15750== Parent PID: 15738
==15750== 
==15750== Conditional jump or move depends on uninitialised value(s)
==15750==    at 0x483DF49: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x49CBD28: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(char const*) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28)
==15750==    by 0x10B401: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B173: Developer::Developer() (employees.hpp:34)
==15750==    by 0x10AE3F: main (main.cpp:31)
==15750== 
==15750== Conditional jump or move depends on uninitialised value(s)
==15750==    at 0x483DF49: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x49CBD28: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(char const*) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28)
==15750==    by 0x10B401: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B1A9: SalesManager::SalesManager() (employees.hpp:52)
==15750==    by 0x10AE4B: main (main.cpp:32)
==15750== 
==15750== 
==15750== HEAP SUMMARY:
==15750==     in use at exit: 404 bytes in 4 blocks
==15750==   total heap usage: 25 allocs, 21 frees, 107,949 bytes allocated
==15750== 
==15750== 101 bytes in 1 blocks are definitely lost in loss record 1 of 4
==15750==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x10B3EB: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B173: Developer::Developer() (employees.hpp:34)
==15750==    by 0x10AE3F: main (main.cpp:31)
==15750== 
==15750== 101 bytes in 1 blocks are definitely lost in loss record 2 of 4
==15750==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x10B3EB: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B1A9: SalesManager::SalesManager() (employees.hpp:52)
==15750==    by 0x10AE4B: main (main.cpp:32)
==15750== 
==15750== 101 bytes in 1 blocks are definitely lost in loss record 3 of 4
==15750==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x10B3EB: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B173: Developer::Developer() (employees.hpp:34)
==15750==    by 0x10C6DE: operator>>(std::basic_ifstream<char, std::char_traits<char> >&, EmployeesArray&) (employees.cpp:196)
==15750==    by 0x10AC75: load(EmployeesArray&) (main.cpp:13)
==15750==    by 0x10AEC7: main (main.cpp:41)
==15750== 
==15750== 101 bytes in 1 blocks are definitely lost in loss record 4 of 4
==15750==    at 0x483B583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==15750==    by 0x10B3EB: Employee::Employee() (employees.cpp:9)
==15750==    by 0x10B1A9: SalesManager::SalesManager() (employees.hpp:52)
==15750==    by 0x10C6EA: operator>>(std::basic_ifstream<char, std::char_traits<char> >&, EmployeesArray&) (employees.cpp:197)
==15750==    by 0x10AC75: load(EmployeesArray&) (main.cpp:13)
==15750==    by 0x10AEC7: main (main.cpp:41)
==15750== 
==15750== LEAK SUMMARY:
==15750==    definitely lost: 404 bytes in 4 blocks
==15750==    indirectly lost: 0 bytes in 0 blocks
==15750==      possibly lost: 0 bytes in 0 blocks
==15750==    still reachable: 0 bytes in 0 blocks
==15750==         suppressed: 0 bytes in 0 blocks
==15750== 
==15750== Use --track-origins=yes to see where uninitialised values come from
==15750== For lists of detected and suppressed errors, rerun with: -s
==15750== ERROR SUMMARY: 8 errors from 6 contexts (suppressed: 0 from 0)

[golovin.valeriy]

Надо придумать как объединить ввод из файла и из терминала.
Также не уверен, что стало лучше с функцией Reed для которой теперь перед вводом надо выделять память, а затем передавать указатель в список работников.

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

Replying to golovin.valeriy:

Надо придумать как объединить ввод из файла и из терминала.
Также не уверен, что стало лучше с функцией Reed для которой теперь перед вводом надо выделять память, а затем передавать указатель в список работников.

Объединить можно двумя перегрузками функции.
Функция Read могла бы сама выделить память и отдать указатель.

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

Поле total_salary_ -- дополнительная информация, которую нужно поддерживать согласованной. Вряд в этом задании нужна эта оптимизация, требующая дополнительного контроля.

Вам точно нужен метод copy()?

    std::string name;
    int32_t base_salary, has_bonus;
    istr >> name >> base_salary >> has_bonus;
    if (!istr.good())
        return -1;

    *this = Developer(name, base_salary, has_bonus);

Почему бы сразу не заполнить поля this?

Магические константы для типов лучше заменить на enum

7.5/10