WW #11

[Артур Гулецкий (huletski)]

Общие замечания

• не работает функция сохранения (вряд ли вы ее, судя по коду, вообще тестировали);
• формат вывода информации о работниках отличается от требуемого (label'ы текстовых полей, e.g. "Base salary", а не "Base Salary";
• valgrind находит ошибки при запуске на данных из условия;

bin_manip.h

• 53: std::size_t больше подходит для размера;

employees.h

• 8: s/stdint.h/cstdint
• 11: нарушено правило трех (copy ctor и op= не определены и не запрещены);
• 14: методы обычно со строчных букв называют, но если вам так больше нравится - ок;
• 59: в массиве по условию должны храниться неконстантные указатели;

bin_manip.cpp

• 9: используйте списке инициализации в конструкторах;
• 36: если программа запущена на big-endian платформе байты будут записаны в неверном порядке -> нужно писать побайтово. Аналогично для чтения;
• 58: a если в буфер мал оказался? Добавьте код, который в этом случае устанавливает failbit, выходит из цикла, в конец буфера нуль-терминатор пишет;
• 68: не хватает инвалидации потока, если считали не 0 и не 1;
• 74: переместите реализацию ввода-вывода сотрудников в emploees.cpp; используйте полиморфизм, а не диспетчеризацию по id и dynamic_cast (использование которого зачастую само по себе является поводом задуматься о том, чтобы переписать код);
• 79: не хватает отступов, т.к. строка содержит продолжение statement'a;
• 94: при итерации по массиву лучше использовать std::size_t в качестве типа индекса;

employees.cpp

• 30: списки инициализации;
• 85: std::memcpy;
• 118: дублирование кода, который определяет класс-наследник Employee по числовому значению (которые лучше вынести в константы). Почему это плохо в контексте задания обсудили на практике;
• 133: полиморфизм.

Баллы: 4.5, доделывайте.

[yakovlev.aleksandr]

Вроде поправил что успел...

[Артур Гулецкий (huletski)]

Решение собирается с warnings -> -0.5:

{lab_11}[2110]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11
Updating '.':
At revision 2773.
M       CMakeLists.txt
{lab_11}[2111]$ svn diff
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt	(revision 2773)
+++ CMakeLists.txt	(working copy)
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.10.2)
+cmake_minimum_required(VERSION 3.5)
 
 project(lab)
 
@@ -17,4 +17,4 @@
         include/employees.h
         include/bin_manip.h)
 
-add_executable(lab_11 ${SOURCE} ${HEADER})
\ No newline at end of file
+add_executable(lab_11 ${SOURCE} ${HEADER})
{lab_11}[2112]$ mkdir build && cd build
{build}[2113]$ cmake ..
-- The C compiler identification is GNU 9.2.1
-- The CXX compiler identification is GNU 9.2.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/g++
-- Check for working CXX compiler: /usr/bin/g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11/build
{build}[2114]$ make
Scanning dependencies of target lab_11
[ 25%] Building CXX object CMakeFiles/lab_11.dir/src/main.cpp.o
[ 50%] Building CXX object CMakeFiles/lab_11.dir/src/employees.cpp.o
/home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11/src/employees.cpp: In function ‘std::istream& operator>>(std::istream&, EmployeesArray&)’:
/home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11/src/employees.cpp:184:1: warning: no return statement in function returning non-void [-Wreturn-type]
  184 | }
      | ^
/home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11/src/employees.cpp: In static member function ‘static Employee* Employee::ByType(int)’:
/home/hfx/dvl/cpp19/yakovlev.aleksandr/lab_11/src/employees.cpp:41:1: warning: control reaches end of non-void function [-Wreturn-type]
   41 | }
      | ^
[ 75%] Building CXX object CMakeFiles/lab_11.dir/src/bin_manip.cpp.o
[100%] Linking CXX executable lab_11
[100%] Built target lab_11

Если их починить, данные все так же выводятся в неверном формате (бонус, должность Sales Manager, label для базовой заработной платы) -> -1.5.
Если починить и это, тесты проходят успешно.

Замечания по коду:

• bin_manip.cpp. используйте списки инициализации;
• bin_manip.cpp:35, 58. чтение/запись int32_t в little-endian реализована неверно. Байты записываете в порядке, в котором они лежат в памяти, а он зависит от платформы. Нужно было брать _значение_ (для которого порядок гарантирован), читать из него соотвествующий байт (сдвиг + & 0xFF) и записывать результат -> -1;
• bin_manip.cpp: отсутствует обработка ошибок (установка failbit) в коде считывания данных (bool - если считалось значение отличное от 0 и 1, c-string - не был найден нуль-терминатор, в этом случае решение вообще зависнет) -> -1;
• employees.cpp:80. отступы. Например:

   ifs >> foo >> bar
       >> baz;
  

• employees.cpp:147. странно, что метод Add удаляет аргумент, так как неясно, нужен ли еще объект коду, который вызвал Add.

+ часть замечаний из предыдущего комментария.

За мелкие замечания совокупно -0.5.

Баллы: 5.5.