Opened 3 years ago
Closed 3 years ago
#683 closed ожидается проверка (задача сдана)
HW #2 (Huffman) Garaev Timur
Reported by: | Garaev Timur | Owned by: | Антон Филатов |
---|---|---|---|
Component: | HW #3 (Huffman) | Version: | 2.0 |
Keywords: | Cc: |
Description
Сейчас готово:
0) Настроен и запускается DOCTEST.
1) Параметры парсятся, плюются исключениями.
2) Дерево правильно строится по набору пар (частота-буква) и возвращает правильный набор кодов.
3) Написан Makefile, который вроде красиво собирает это в два исполняемых файла (если есть идеи, как можно это улучшить, я с радостью бы послушал).
В целом посылка, чтобы понять, на правильном ли я пути, есть ли какие-то фундаментальные ошибк в проектировании и где можно сделать красивее/правильнее.
Change History (6)
comment:1 Changed 3 years ago by
Type: | ожидается проверка → ожидаются исправления |
---|
comment:2 Changed 3 years ago by
Type: | ожидаются исправления → ожидается проверка |
---|---|
Version: | 1.0 → 2.0 |
Тесты говорят, что все работает как нужно. Надеюсь на больших тестах тоже.
У себя локально потестил картинки/большие тексты, вроде работает как нужно.
Тесты поправил, тестируются все методы, которые используются. Они все еще одним полотном из testcase'ов, но я честно не понимаю, почему это нужно чинить. Мне кажется это довольно читаемо и понятно, где что тестируется.
Еще ругается валгринд на что-то связанное с STL-ными контейнерами. Я не уверен, что это какой-то мой косяк. Но память у приложения не течет. Все что создается -- чистится.
comment:3 Changed 3 years ago by
Type: | ожидается проверка → ожидаются исправления |
---|
Так, ну падает сейчас много всего на самом деле.
Вот пример файла, который вы архивируете вроде бы правильно, но на выходе получаете пустой файл
$ 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
Стиль: 5
Тесты: 4
comment:5 Changed 3 years ago by
Type: | ожидаются исправления → ожидается проверка |
---|
Увеличил покрытие тестами.
Научился генерировать Test case'ы с помощью макросов DOCTEST'а, чтобы не копировать много всяких одинаковых строчек в тестах.
Исправил почти все замечания по стилю. Тесты, про которые вы сказали (и еще парочку других) теперь отрабатывают правильно.
comment:6 Changed 3 years ago by
Resolution: | → задача сдана |
---|---|
Status: | assigned → closed |
не обрабатываете флаги --output и --file,
остальные тесты проходят
Корректность: 13/15
Стиль: 10/10
Тесты: 5/5
По дереву выглядит правдоподобно, точно протестировать пока не могу
То, что вы прикрутили doctest - это хорошо. Но всё же правильной структуры классов не хватает. Понятно,что можно всё сделать в одно полотно TEST_CASE-ами, но хотелось бы обернуть это в более читабельную форму. Ну и покрытие тестами надо будет сделать очень внимательно - каждый метод каждого класса, если это возможно, надо проверить. Причём нельзя делать проверку метода А через метод Б, если метод Б не проверен
Makefile у вас хороший.
Общее замечание: добавьте namespace.
В целом выглядит уже очень достойно