HW #3

[Egor Suvorov]

Проверялась ревизия 4014 или ранее.

Третья попытка разблокирована.

Корректность 4/9

• Не проходят ваши собственные тесты:

      Running tests from ../../../cpp19/bagryanova.ekaterina/hw_03/test_hw_03
  [doctest] doctest version is "2.3.7"
  [doctest] run with "--help" for options
  ===============================================================================
  /home/osboxes/cpp2019/cpp19/bagryanova.ekaterina/hw_03/test/TestHuffman.cpp:5:
  TEST CASE:  Huffman tests
  
  /home/osboxes/cpp2019/cpp19/bagryanova.ekaterina/hw_03/test/TestHuffman.cpp:16: ERROR: CHECK( symbols_code['a'].size() == 1 ) is NOT correct!
    values: CHECK( 2 == 1 )
  
  /home/osboxes/cpp2019/cpp19/bagryanova.ekaterina/hw_03/test/TestHuffman.cpp:20: ERROR: CHECK( symbols_code['c'].size() == 3 ) is NOT correct!
    values: CHECK( 2 == 3 )
  
  /home/osboxes/cpp2019/cpp19/bagryanova.ekaterina/hw_03/test/TestHuffman.cpp:21: ERROR: CHECK( symbols_code['d'].size() == 3 ) is NOT correct!
    values: CHECK( 2 == 3 )
  
  ===============================================================================
  [doctest] test cases:      5 |      4 passed |      1 failed |      0 skipped
  [doctest] assertions:     65 |     62 passed |      3 failed |
  [doctest] Status: FAILURE!
  

• На случайном пятимегабайтном файле у меня сжатие занимает 4.87, разжатие — 13.85.
• Не работает под Windows: файлы надо открывать в бинарном режиме.

Тесты 5/8

• Ваши тесты не проходят
• Тесты на ByteWriter: лучше загляните напрямую в stringstream при помощи str() и сравните ответ при помощи одного ==.
• Тесты должно быть просто читать и проверять глазами. Если там встречается два вложенных цикла, это сложно. В write code сделайте честную list-init переменной code вместо циклов. Используйте битовые литералы вроде 0b10'0000'00` вместо сдвигов.
• Тест Хаффмана маловато случаев покрывает (например, когда файл пустой). И ещё стоит проверять прям сами коды символов. И статистику проверить посильнее (а то как раз с ней проблемы).

Архитектура 3/5

• ByteWriter — зачем публичный out, buffer, statistics? Стоит закрыть и сделать геттеры. Аналогичная претензия к HuffmanArchiver.
  • is_additional плохо согласуется со статистикой (вероятно, из-за этого у вас и проблемы): "дополнительными или нет" могут быть только биты, а статистику вы считаете по байтам, причём характер байта определяется последним записанным битом. Это странно. Кажется, что надо либо статистику считать по битам, либо создавать ByteWriter с фиксированным видом байт.
  • В деструкторе ByteWriter стоит поставить assert, что буфер пуст. Иначе пользователь может забыть его очистить.
• Trie::add_node — в чём сакральный смысл передавать deque<bool> по ссылке? add_node не возвращает в нём никакого значения. Если это нужно для оптимизации, то возьмите deque<> по значению, а при вызове сделайте move, явно показав, что вам неважно значение deque после вызова.
• Trie::go — кажется, это либо всё-таки метод TrieNode и не использует root, либо не должно быть первого параметра. И тоже очень странно, что второй параметр мутабельный. Судя по тому, как это используется в тестах, после go() мы всегда ожидаем, что пришли в вершину со значением => это звучит как инвариант.
• HuffmanTree
  • Странно, что можно создать HuffmanNode от одной вершины.
• Кажется, что в HuffmanArchiver появился кусок логики ввода-вывода и подсчёта статистики (read_symbol, write_symbol). Он не имеет никакого отношения к сжатия по Хаффману.

Стиль 4/8

• В CLIException помог бы using конструктора.
• В TestCLI.cpp лучше не CLI_checks (тем более что название не очень соответствует snake_case или CamelCase), а честно написать три CHECK в каждом тесте.
• В main.cpp незачем обрабатывать std::exception и CLIException по-разному. Второе является наследником первого.
• main.cpp: лучше assert на парсинг условий поставить сразу после создания cli. А ещё лучше внести это в инвариант класса CLI и проверить в конструкторе CLI.
• В Statistics.hpp не хватает size_t
• Не используйте endl внутри operator<<. Он сбрасывает буфер, это пользователь должен делать сам.
• write_all — pad_to_byte (добей до байта)
• В Trie tests лучше сделать deque<bool> x = {0, 1, 0, 0, 0, 1}. Или хотя бы одинаково инициализировать code_a и bin_text. А вообще промежуточные переменные code_a, конечно, не нужны.
• TestTrie: мойте тарелки перед едой ​http://blog.algoprog.ru/init-not-clear/!
• В Huffman.cpp (и в остальных местах) не пытайтесь сделать длинный отступ перед : и ,, а просто сделайте перенос строки перед : и обычный отступ.
• HuffmanNodeComparator — не хватает слова "по весу"
• Вместо element.first/element.second используйте structured binding в range-based-for
• Переменная new_node не нужна.
• Заглушаются ошибки в конструкторе HuffmanTree: а что если ни один из ифов не сработает? Должен быть хотя бы assert на все технически возможные случаи.
• go_and_find_code разумно брать cur_code по ссылке.
• В find_symbols_code зря разобран случай root == nullptr, он должен разобраться автоматически.
• В HuffmanArchiver очень напрягает, что в compress записи байт перемежаются с использованием ByteWriter. Неочевидно, что не распилим биты где-то в середине. Чтобы это точно не было проблемой, создавайте ByteWriter только в самых вложенных местах, где используется только он.
• Не хватает assert'а в extract про то, что прочитали либо '0' (стоит иф), либо '1' (хочу assert).
• Лишние пустые строчки внизу в HuffmanArchiver`

[Egor Suvorov]

P.S. А ещё перепроверьте, пожалуйста, наличие слов final, noexcept, explicit (это в стиль).

[Bagryanova Ekaterina]

Дедлайн по исправлению пятница 22.59