HW #3

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

/home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test/TestMain.cpp:7:10: fatal error: doctest.h: No such file or directory

7 | #include <doctest.h>

вы пакет с doctest в cmake подтянули, но к таргетам с тестами не прицепили (для system-wide не обязательно, но раз вы используете cmake find_package -- то, по-хорошему, надо.)
target_link_libraries(...... doctest::doctest)
у doctest, находимого с помощью cmake, путь к заголовку: <doctest/doctest.h>

CHECK(size - n == 1e6 / 8);
слева int, справа -- double. Сравнение будет в double. Сравнивать double на точное равенство -- затея, обреченная на провал почти всегда. Два одинаковых числа могут быть не равны.

add_definitions("-D_GLIBCXX_DEBUG зачем вам перманентно включать дебажный режим стандартной библиотеки? Для дебажных сборок запускайте cmake -DCMAKE_BUILD_TYPE=Debug

Тесты протекли

==1492== 64 (32 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 12
==1492==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1492==    by 0x14E1E3: plant(std::priority_queue<HuffmanNode*, std::__debug::vector<HuffmanNode*, std::allocator<HuffmanNode*> >, compare_by_frequency>&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x14E64D: HuffmanTree::zip(std::__debug::unordered_map<char, int, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, int> > >&, std::__debug::unordered_map<char, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&, int&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x149008: HuffmanArchiver::compress(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123313: _DOCTEST_ANON_FUNC_6() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x1224EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123234: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492== 
==1492== 64 (32 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 12
==1492==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1492==    by 0x14E1E3: plant(std::priority_queue<HuffmanNode*, std::__debug::vector<HuffmanNode*, std::allocator<HuffmanNode*> >, compare_by_frequency>&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x14E64D: HuffmanTree::zip(std::__debug::unordered_map<char, int, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, int> > >&, std::__debug::unordered_map<char, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&, int&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x149008: HuffmanArchiver::compress(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123B9A: _DOCTEST_ANON_FUNC_10() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x1224EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123234: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492== 
==1492== 64 (32 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 7 of 12
==1492==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1492==    by 0x149457: HuffmanArchiver::extract(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123BC6: _DOCTEST_ANON_FUNC_10() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x1224EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123234: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492== 
==1492== 16,352 (32 direct, 16,320 indirect) bytes in 1 blocks are definitely lost in loss record 12 of 12
==1492==    at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1492==    by 0x14E1E3: plant(std::priority_queue<HuffmanNode*, std::__debug::vector<HuffmanNode*, std::allocator<HuffmanNode*> >, compare_by_frequency>&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x14E64D: HuffmanTree::zip(std::__debug::unordered_map<char, int, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, int> > >&, std::__debug::unordered_map<char, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<char>, std::equal_to<char>, std::allocator<std::pair<char const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&, int&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x149008: HuffmanArchiver::compress(std::istream&, std::ostream&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123EAE: _DOCTEST_ANON_FUNC_12() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x1224EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==1492==    by 0x123234: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)

На пустом файле не работает.

Обработка аргументов командной строки допускает некорректные ключи.

    bool zip;
    std::string fin_name, fout_name;

    CLI::set_flags(zip, fin_name, fout_name, argc, argv);

Инициализируйте переменные. zip может быть неинициализирован -> UB.

Под windows файлы будут испорчены.

#include <sys/stat.h> вот давайте без платформоспецифичных заголовков. Посчитать размер файла не такая уж и сложная задача.

Статическое поле класса для корня дерева, вы уверены, что это хорошая идея? А если нужно параллельно несколько разных файлов обработать?

Используйте умные указатели. Не надо мучать память вручную.

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

Все методы статические и глобальное состояние -- в худших традициях Cи.

У вас только end-to-end тесты.

6 + 2 + 1 + 5

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

CMake Error at CMakeLists.txt:13 (add_executable):
  Cannot find source file:

    test/TestCli.h

Недокоммитили

---

• Файлы нужно открывать в бинарном режиме. Иначе под windows будет получаться мусор.
• Почему бы парсеру аргументов не вернуть структуру с распарсенными значениями?
• Почему бы методам compress/extract не вернуть структуру с посчитанной статистикой, а не делать это вручную в main?
• вложенность кода в main можно уменьшить с помощью function-try-block
• в случае ошибки программа не должна возвращать 0
• в флаге --file ошибка.
• Построение priority_queue не должно быть частью публичного интерфейса. Это детали реализации конструктора дерева.

•         tree.add_HuffmanNode("00", 'a');
          tree.add_HuffmanNode("01", 'b');
  

Этот метод может нарушить свойство префиксности кода. Лучше его убрать и предоставить конструктор, принимающий таблицу кодов (и кидающий исключение, если код не префиксный)

• Размещайте публичные поля и методы перед приватными. Приватные кишки никому не нужны.
• Логику перемещения по дереву (указатель на текущую вершину и т.д.) при декодировании лучше вынести в отдельный класс, с конструктором, принимающим дерево.
• static int32_t zip(std::unordered_map<char, int>& letters, std::unordered_map<char, std::string>& dict); Что делает эта функция? Из названия совершенно не понятно. Видимо, строит таблицу кодов. Но почему таблица частот передается по изменяемой ссылке? Почему бы этому методу не вернуть таблицу кодов через возвращаемое значение, а не менять аргументы?

•     explicit HuffmanTree(my_queue HuffmanNode_queue);
  
      static my_queue priority_queue_with_HuffmanNodes(
              const std::unordered_map<char, int>& letters);
  
  

Зачем в публичном интерфейсе промежуточный шаг с очереднью? Почему бы конструктору не принимать таблицу частот?

• Функции compress/extract надо разбить на отдельные функцкии, выполняющие законченное действие:
  • Построение таблицы частост для входного потока
  • Построение дерева по этой таблице
  • Кодирование входного потока

•    // remember stuff
      fout.write(reinterpret_cast<const char*>(&n), sizeof(n));
      fout.write(reinterpret_cast<const char*>(&freq_sum), sizeof(freq_sum));
  
      for (const auto&[ch, k] : dict)
          fout << ch << k << ' ';
  

Не надо смешавать бинарный и текстовый ввод/вывод. Это редко приводит к чему-то хорошему.

• Побитовое чтение/запись лучше выделить в отдельный класс-обертку над потоком.

---

7.5 + 5 + 3 + 6