HW #3

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

что-то у вас тесты текут

==22926== 48 (24 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 8
==22926==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==22926==    by 0x13A2C5: HuffmanTree::makeHuffmanTree() (Huffman.cpp:29)
==22926==    by 0x136908: _DOCTEST_ANON_FUNC_2() (TestHuffman.cpp:6)
==22926==    by 0x11ED42: doctest::Context::run() (doctest.h:5878)
==22926==    by 0x11F75F: main (doctest.h:5962)
==22926== 
==22926== 48 (24 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 8
==22926==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==22926==    by 0x13A2C5: HuffmanTree::makeHuffmanTree() (Huffman.cpp:29)
==22926==    by 0x13765A: _DOCTEST_ANON_FUNC_4() (TestHuffman.cpp:23)
==22926==    by 0x11ED42: doctest::Context::run() (doctest.h:5878)
==22926==    by 0x11F75F: main (doctest.h:5962)
==22926== 
==22926== 72 (24 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 7 of 8
==22926==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==22926==    by 0x13A383: HuffmanTree::makeHuffmanTree() (Huffman.cpp:39)
==22926==    by 0x136D14: _DOCTEST_ANON_FUNC_2() (TestHuffman.cpp:12)
==22926==    by 0x11ED42: doctest::Context::run() (doctest.h:5878)
==22926==    by 0x11F75F: main (doctest.h:5962)
==22926== 
==22926== 72 (24 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 8 of 8
==22926==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==22926==    by 0x13A383: HuffmanTree::makeHuffmanTree() (Huffman.cpp:39)
==22926==    by 0x137CB8: _DOCTEST_ANON_FUNC_4() (TestHuffman.cpp:31)
==22926==    by 0x11ED42: doctest::Context::run() (doctest.h:5878)
==22926==    by 0x11F75F: main (doctest.h:5962)
==22926== 

На пустом файле все упало. В логе отрицательные размеры.

При разархивировании файл становится больше, чем исходный.

В статистику может выводиться неправильный размер исходного файла.

На приложенном файле не работает архивирование-разархивирование.

Прекрасный скопипащенный дважды текст в тестах поместите в переменную.

Протестируйте на бинарных файлах.

    try {
        cli.parseFlags();
    } catch (const ArgumentException& e) {
        std::cout << e.what() << std::endl;
        flag = false;
    }
    if (flag) {

А зачем булев флаг? (лучше давать осмысленное имя, а не просто flag) Вам просто нужно сделать
return EXIT_FAILURE, а не увеличивать уровень вложенности кода.

У вас две функции: doCompress() и doExtract(). Что нужно делать, если обе вернут true? Функция для выбора одного из двух взаимоисключающих действий должна быть одна.

Расставьте const у методов, где это уместно.

Конструктор дерева не создает готовое к использованию дерево. Требуется дополнительный вызов метода makeHuffmanTree. Вместо деструктора -- вызов другого метода. Как-то это не правильно. не по-RAII.

class Compare {
public:
    bool operator() (HuffmanNode*& a, HuffmanNode*& b) {
        return a->freq > b->freq;
    }
};

Зачем ссылки?

---

Детально код не проверял, поскольку пока не работает. Только самое заметное.
Используйте умные указатели, не надо мучать память вручную. Захватывайте ресурсы и делайте готовый к работе объект в конструкторе, освобождайте в деструкторе.

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

   bool operator() (HuffmanNode* a, HuffmanNode* b) {
        return a->freq > b->freq;
    }

оператор нужно пометить const. Иначе не обязано компилироваться.

priority_queue -- это внутренность дерева, нужная только для его построения. Класс дерева не должен его отдавать никому. Для итогового представления дерева она не нужна. Лучше и достаточно только указателя на корень.

---

По условию нужен двухпроходной алгоритм без чтения всего файла в память.

Все файлы надо открывать в бинарном режиме.

Сложную вложенность с перехватом исключений можно упростить с помощью ​https://en.cppreference.com/w/cpp/language/function-try-block

 if (i == argc - 1 || !strcmp(argv[i + 1], "-c") || !strcmp(argv[i + 1], "-u") ||
                !strcmp(argv[i + 1], "-f") || !strcmp(argv[i + 1], "--flag") || !strcmp(argv[i + 1], "-o") ||
                !strcmp(argv[i + 1], "--output")) {

Поскольку после флага -f / --file может быть только имя файла, все, кроме первого условия (i == argc -1) не нужно. "-c" -- вполне себе законное имя файла в unix! Для флага -o то же самое.

   extr.extract(inputStream, outputStream);
   return extr.getInfoSize();

Можно в методе extract так же вернуть размер. Хранить его полем класса особой необходимости нет.

Чтение по одному биту с формированием строки и проверка вхождения этой строки в таблицу кодов после каждого бита -- очень неэффективно. Лучше использовать дерево: переходить от узла к узлу и выдавать байт, если достигли листа. Эту логику можно завернуть в какой-нибудь класс TreeWalker
c методом std::optional<byte> Consume(bit)

---

Для построения дерева не нужен весь исходный файл. Нужна только таблица частот. Логику ее построения стоит держать отдельно от дерева.

---

Тесты довольно слабые. Только дерево с 2-3 вершинами (с учетом корневой). Так совсем не проверить, что коды получаются верными.
Еще нужны тесты кодирования/раскодирования одного символа с помощью построенного дерева.
И тесты чтения и записи вспомогательной информации.
Читалки битов также можно отделить и протестировать.

---

На последней версии: проблемы со статистикой и длиной кода (на ранее приложенном файле воспроизводится). Длина сжатых байтов (без доп информации) не может получаться больше чем длина исходного файла из-за правила построению дерева.

---

5 + 3 + 1.5 + 6