WW #11

[Kreslavski Kirill]

Я написал что-то относительно разумное, но оно пока не особо работает. Ошибок при работе с памятью вроде пока нет, но работу с файлами я ещё не проверял. Я видимо как-то не правильно написал оператор для вывода EmployeeArray?, поэтому у меня сейчас выводятся указатели на элементы, а не выводятся они сами.
Пофикшу всё при первой же возможности, но не раньше воскресенья.

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

Я написал что-то относительно разумное, но оно пока не особо работает. Ошибок при работе с памятью вроде пока нет, но работу с файлами я ещё не проверял. Я видимо как-то не правильно написал оператор для вывода EmployeeArray??, поэтому у меня сейчас выводятся указатели на элементы, а не выводятся они сами.

Да как-то вообще не работает (даже exit):

{lab_11}[2190]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/kreslavskiy.kirill/lab_11
Updating '.':
At revision 2633.
{lab_11}[2191]$ make
mkdir -p obj
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/bin_manip.o src/bin_manip.cpp
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/main.o src/main.cpp
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/employees.o src/employees.cpp
g++ obj/bin_manip.o obj/main.o obj/employees.o -o lab_11 
{lab_11}[2192]$ ./lab_11 
add 2 Joe 100 20 300
add 1 Billy 1000 1
list
exit
^C

Похоже, проблема возникает, если вводить данные о SalesManager?'e.
Перед следующей попыткой протестируйте решение на данных из условия.
Так как решение слишком недоделано, код просмотрю по диагонали.

Замечания:

Misc

• нарушено "правило трех" (e.g. для Employee сгенерированные реализации op=, copy-ctor будут работать неверно. Методы нужно либо реализовать вручную, либо запретить);
• employees.h:33. Суффиксы _dev, _sm в наследниках не нужны. Странно, что методы чтения/записи невиртуальные (с виртуальной реализацией проще добавить поддержку такого кода: Employee *e = ...; stream << *e;; op<<(std::ostream& os, const Employee &e) вызывает e.print(os));

main.cpp

35: считывайте в std::string изпользуя op>>; перемнные лучше объявлять рядом с местом первого использования;
40: loop-and-a-half. Можно переписать (чтобы убрать дублирование чтения) так:

while (true) {
  std::cin >> command;
  if (command == "exit") { break; }
  ...
}

54: этот код можно вынести в перегрузку op>> для EmployeesArray (бинарные ввод);

bin_manip.cpp

7: что такое 4? если программа запущена на big-endian платформе порядок байтов будет записан неверно -> нужно писать байты инта по одному, чтобы гарантировать порядок;
17: можно не проверять, если поток fail вызов read проигнорируется. Кроме того, не стоит печатать сообщения, если они не отладочные: в манипуляторе (очень базовый, общий код) ошибку лучше обозначить (см. след. пункт) установкой бита состояния потока, а ее обработкой (игнорирование, печать сообщения, whatever) пусть занимается более высокоуровневый код;
50: можно добавить блок else, который устанавливает failbit; в op>> для read_c_str тоже не хватает установки failbit'ов, если формат отличен от ожидаемого;

employees.cpp

1, 2: cstring, cstdio
9: используйте списки инициализации;
27: используйте написанный манипулятор для чтения c-string;
36: strncpy;
44: strncpy(_name, s_tmp.c_str(), ...); напомню, что измененное условие позволяет хранить имена в std::string;
62, 63: os << (_has_bonus ? '+' : '-') << std::endl выглядит лаконичнее;
149: дублирование кода определения типа и создания объекта по числовому коду (похожий код в main.cpp). Почему это особенно плохо в контексте лабораторной обсудили на практике;

---

Баллы: 1.5 (что-то похожее все-таки реализовано), доделывайте.

Пофикшу всё при первой же возможности, но не раньше воскресенья.

Еще 7 дней, по условию продления дедлайна. Обязательно перепроверьте решение на примере из условия перед посылкой.
Обратите внимание, что однотипные повторяющиеся замечания я не писал.

[Kreslavski Kirill]

Приложив неимоверные моральные усилия и переписав код раз 5 я всё-таки смог добиться полной работоспособности написанного. Причём оно даже работает корректно, не выдаёт ошибок в валгринде и выглядит приличнее чем предыдущая версия.

[Kreslavski Kirill]

Я таки смог найти баг, из-за которого ловил segmentation fault. Там был цикл, который работал до _employee.size() + added_amount . А внутри этого цикла я добавлял элементы в вектор пушбэкая, тем самым динамически пересчитывался _employee.size() и я пытался считать что-то лишний раз, а считывать было неоткуда. Сейчас я залил в svn нормальную версию, которая работает и имеет одну точку принятия решения. failbit пока не понял как ставить, может быть допилю чуть позже (завтра или сегодня ночью)

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

На примере из условия решение неверно работает с памятью, неверно записывается список сотрудников -> -2.5

{lab_11}[2016]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/kreslavskiy.kirill/lab_11
Updating '.':
At revision 2788.
{lab_11}[2017]$ make
mkdir -p obj
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/bin_manip.o src/bin_manip.cpp
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/main.o src/main.cpp
g++ -O2 -Wall -Werror -std=c++11 -Iinclude -g -c -MMD -o obj/employees.o src/employees.cpp
g++ obj/bin_manip.o obj/main.o obj/employees.o -o lab_11 
{lab_11}[2018]$ valgrind ./lab_11 
==3843== Memcheck, a memory error detector
==3843== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==3843== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==3843== Command: ./lab_11
==3843== 
add 2 Joe 100 20 300
add 1 Billy 1000 1
list
1. Sales Manager
Name: Joe
Base Salary: 100
Sold items: 20
Item price: 300
2. Developer
Name: Billy
Base Salary: 1000
Has bonus: +
== Total salary: 2160

save example.edb
load example.edb
list
1. Sales Manager
Name: Joe
Base Salary: 100
Sold items: 20
Item price: 300
2. Developer
Name: Billy
Base Salary: 1000
Has bonus: +
3. Sales Manager
Name: Joe
Base Salary: 100
Sold items: 20
Item price: 300
4. Developer
Name: Billy
Base Salary: 1000
Has bonus: +
== Total salary: 4320

save merged.edb
exit
==3843== 
==3843== HEAP SUMMARY:
==3843==     in use at exit: 73,384 bytes in 15 blocks
==3843==   total heap usage: 25 allocs, 10 frees, 101,688 bytes allocated
==3843== 
==3843== LEAK SUMMARY:
==3843==    definitely lost: 104 bytes in 4 blocks
==3843==    indirectly lost: 576 bytes in 10 blocks
==3843==      possibly lost: 0 bytes in 0 blocks
==3843==    still reachable: 72,704 bytes in 1 blocks
==3843==         suppressed: 0 bytes in 0 blocks
==3843== Rerun with --leak-check=full to see details of leaked memory
==3843== 
==3843== For counts of detected and suppressed errors, rerun with: -v
==3843== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
{lab_11}[2019]$ hexdump -C merged.edb 
00000000  04 00 00 00 02 00 00 00  4a 6f 65 00 64 00 00 00  |........Joe.d...|
00000010  14 00 00 00 2c 01 00 00  01 00 00 00 42 69 6c 6c  |....,.......Bill|
00000020  79 00 e8 03 00 00 31 02  00 00 00 4a 6f 65 00 64  |y.....1....Joe.d|
00000030  00 00 00 14 00 00 00 2c  01 00 00 01 00 00 00 42  |.......,.......B|
00000040  69 6c 6c 79 00 e8 03 00  00 31                    |illy.....1|
0000004a

По коду:

• нарушено rule of 3 для классов employees.h (copy ctor, op=, сгенерированные компилятором, работают неверно) -> -0.5;
• main.cpp:3. можно было хранить на стеке; выделенная память не освобождается;
• bin_manip.cpp:19, 43. отсутствует обработка ошибок (установка failbit) в коде считывания данных (bool - если считалось значение отличное от 0 и 1, c-string - не был найден нуль-терминатор -> -1;
• bin_manip.cpp:35. можно было записать std::strlen + 1 символов за один вызов write;
• employees.cpp:38. std::strncpy + std::string::c_str вместо поэлементного копирования;

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

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

Баллы: 5.25.