HW #2 (Huffman) Garaev Timur

[Garaev Timur]

Сейчас готово:

0) Настроен и запускается DOCTEST.
1) Параметры парсятся, плюются исключениями.
2) Дерево правильно строится по набору пар (частота-буква) и возвращает правильный набор кодов.
3) Написан Makefile, который вроде красиво собирает это в два исполняемых файла (если есть идеи, как можно это улучшить, я с радостью бы послушал).

В целом посылка, чтобы понять, на правильном ли я пути, есть ли какие-то фундаментальные ошибк в проектировании и где можно сделать красивее/правильнее.

[Антон Филатов]

По дереву выглядит правдоподобно, точно протестировать пока не могу
То, что вы прикрутили doctest - это хорошо. Но всё же правильной структуры классов не хватает. Понятно,что можно всё сделать в одно полотно TEST_CASE-ами, но хотелось бы обернуть это в более читабельную форму. Ну и покрытие тестами надо будет сделать очень внимательно - каждый метод каждого класса, если это возможно, надо проверить. Причём нельзя делать проверку метода А через метод Б, если метод Б не проверен
Makefile у вас хороший.

Общее замечание: добавьте namespace.

В целом выглядит уже очень достойно

[Garaev Timur]

Тесты говорят, что все работает как нужно. Надеюсь на больших тестах тоже.
У себя локально потестил картинки/большие тексты, вроде работает как нужно.

Тесты поправил, тестируются все методы, которые используются. Они все еще одним полотном из testcase'ов, но я честно не понимаю, почему это нужно чинить. Мне кажется это довольно читаемо и понятно, где что тестируется.

Еще ругается валгринд на что-то связанное с STL-ными контейнерами. Я не уверен, что это какой-то мой косяк. Но память у приложения не течет. Все что создается -- чистится.

[Антон Филатов]

Так, ну падает сейчас много всего на самом деле.
Вот пример файла, который вы архивируете вроде бы правильно, но на выходе получаете пустой файл

$ hexdump -C x.1.core
00000000  78          |x|
00000001

$ cat x.1.core
x

Кажется, что вообще все тесты, где текст состоит только из одной буквы (даже если она несколько раз повторяется), то вы возвращаете пустой файл

Когда на вход приходит пустой файл, вы падаете с сегфолтом - а надо вернуть нули и пустой файл

При этом если в тексте есть хотя бы две разные буквы, не считая валгринда, всё отрабатывает нормально.

И да, валгринд что-то очень ругается. Он не может ругаться из-за внутренней реализации STL контейнеров - они отточены за годы. Скорее всего это ваш косяк, точнее где-то вы не очень аккуратно этими контейнерами пользуетесь.

Про полотно тестов:
представьте, то у вас не 3 класса, а 300. И вы пишете 301-й. И тогда для того, чтобы скомпилировать тесты для нового класса, вам надо скомпилировать все предыдущие тесты. Это проблема номер раз. Проблема номер два - это поиск нужных тестов конкретного класса среди 300. особенно остро эта проблема стоит, если тесты писали разные люди - они их по-разному называли, и поиском по файлу тут не обойтись

да и покрытие у вас пока не очень большое. По-хорошему каждую функцию каждого класса надо тестировать в нескольких юнит тестах.
Ваш TEST_CASE("Test huffman nodes") - это, конечно, хорошо, но, например, get_left вы тестируете только один раз и только на ноль. А вдруг она всегда только 0 возвращает?
Нет теста на пустое дерево, нет тестов на дерево, состоящее из 5 млн вершин. И для таких деревьев хорошо бы не смог тесты на compress-decompress, а прям каждый метод дерева проверить, работает ли он.

Стиль:

• а вам удобно с сырыми указателями работать? может, хотите shared_ptr или unique_pte? вдруг так будет меньше проблем с валгриндом
• реализация некоторых методов поселилась в заголовочный файл
• очень много раз встречается волшебная константа 8
• пожалуйста, выводите размеры файлов в консоль через перевод строки, а не через пробел, а то проверяющая система не готова к вашим пробелам
• используйте std::endl вместо \n

Корректность: 7/15
Стиль: 5/10
Тесты: 3/5

[Антон Филатов]

И вдогонку ещё напоминаю про нэймспейсы

[Garaev Timur]

Увеличил покрытие тестами.
Научился генерировать Test case'ы с помощью макросов DOCTEST'а, чтобы не копировать много всяких одинаковых строчек в тестах.

Исправил почти все замечания по стилю. Тесты, про которые вы сказали (и еще парочку других) теперь отрабатывают правильно.

[Антон Филатов]

не обрабатываете флаги --output и --file,
остальные тесты проходят
Корректность: 13/15
Стиль: 10/10
Тесты: 5/5