HW #03

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

Пустой файл:

==71063== Use of uninitialised value of size 8
==71063==    at 0x10C7EC: HuffmanArchiver::compress(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/hw_03)
==71063==    by 0x10ACAF: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/hw_03)
==71063== 
==71063== Use of uninitialised value of size 8
==71063==    at 0x10C8D0: HuffmanArchiver::compress(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/hw_03)
==71063==    by 0x10ACAF: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/hw_03)
==71063== 

Файл из ровно одного символа: неверный вывод статистики сжатия

1
1
6

Размер полученного файла -> 15

---

Файл из всего 256 различных байт:
Test failed -- expected compressed data to be no longer than source data, got 257 > 256

при перестановке аргументов командной строки может перестать работать.

        std::istream is(cli.input());
        std::ostream os(cli.output());
        assert(is);

Какой assert?! Тут все пропало -- это эксепшон кидать надо

    std::filebuf *input();
    std::filebuf *output();

1. Парсер аргументов не должен открывать файлы.
2. чем вам i/ofstream не угодил, что вы в более низкоуровневую штуку полезли?

std::cout << std::get<0>(result) << std::endl << std::get<1>(result) << std::endl << std::get<2>(result) << std::endl;
Вы 17 стандарт зачем включили? Чтоб продолжать вот этим кошмаром страдать? structure bingings.

Уберите неиспользуемые переменные.

    Tree() : compress_(256) {};

    void build(const std::vector<int> &counts);

Конструктор ничего не делать. Все делает метод build. Зачем такой конструктор? Или зачем метод делает работу конструктора.

Не стоит называть переменные одной буквой l. Эта замечательная буква часто путается с i и 1.

std::pair и std::tuple -- довольно плохое решение, если поля имеют хоть какой-то смысл. Лучше своя структура.

---

Зачем miltiset, а не просто set?

std::for_each(l.bytes().begin(), l.bytes().end(), [&](auto byte) { compress_[byte] += '0'; });
мне кажется, range-based for короче и читаемее.

Адскую логику чтения и записи по одному битику стоит отделить в обертку.

Первый проход с накоплением статистики и последующим сбросом потока в начало стоит вынести в отдельную функцию.

---

куча однобуквенных переменных в методах extract/compress вперемежку с лютой логикой чтения и записи по битику.

Вы распаковываете сжатый файл в строку, а потом выводите в поток? А можно сразу в поток?

---

Чтение и запись дерева не протестированы. Парсер аргументов тоже надо тестировать.
У вас есть всякие TreeWalker?'ы, которых тоже стоит проверить.

---

5.5 + 4.5 + 4 + 5

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

аргументы --file --output не поддерживаются

----------------------

    bool compress_ = 0;
    bool extract_ = 0;

Давайте использовать соответсвующую типу константу: true/false

compress/decompress -- взаимоисключающие действия. Всесто двух флагов/методов в CLI лучше сделать enum.

Все методы CLI можно пометить const.

Вложенность кода в main можно уменьшить с помощью function-try-block ​https://en.cppreference.com/w/cpp/language/function-try-block

    std::vector<std::string> get_tree_for_test(std::istream &is);
    HuffmanTree get_HuffmanTree_for_test(std::istream &is);

Зачем эти публичные методы в классе HuffmanArchiver?? Также зачем в этом классе поле Tree, если дерево конструируется при каждом вызове методов compress/extract?. Почему бы ему не быть локальной переменной?

---

Вместо двух методов go_left/go_right в TreeWalker? удобнее сделать один метод
std::optional<byte> Consume(bit); А запись в выходной поток делать вне этого метода.

std::shared_ptr<HuffmanNode> &HuffmanNode::left()
{
    return l_;
}

Стоит завести константный и неконстантынй методы. Чтобы не возвращать неконстантную ссылку там, где не надо.

---

 binary += ('0' + !!(byte & (1 << bit)));

binary += (byte & (1 << bit)) ? '0' : '1';

Компилятор сможет отоптимизировать такое. Мелкие ручные оптимизации в ущерб читаемости делать не надо.

Константу 256 надо сделать именованной.

---

Не понятно, зачем нужны по два разных класса *Node и *Tree. При том, что Tree деревом не явялется. Как и TreeNode не является его узлом.
Либо названия классов неправильные, либо что-то еще.

---

Однобуквенные поля и переменные с именем l не убрали.

---

8.5 + 8 + 4 + 6.5