HW #3

[Vasily Alferov]

Круть.

• Корректность: 8/9
• Тесты: Их нет, пока 0
• Архитектура: 3/5
• Стиль: 4/8

Замечания.

По корректности:
Единственное замечание -- не работают длинные флаги (--file, --output).
Это минус балл.

По архитектуре:
Буффер, который умеет флашиться -- это круто.
Замечания:

• HuffmanTree наследуется от Node, серьёзно? Зачем? Тем более, что он ещё и композирует указатель на Node.
• Тебе правда будет сильно проще писать тесты, если ты будешь работать с std::istream/std::ostream, а не с std::ifstream/std::ofstream

По стилю:

• {} после членов класса типа unsigned int -- это странно. Лучше уж либо = 0, либо вообще ничего.
• Кстати, unsigned int -- длинное слово, чаще всего проще писать size_t или uint32_t. У последнего, кстати, помимо прочего, гарантирована длина.
• Класс с публичными переменными внутри -- это плохо, скорее всего, это беды с инкапсуляцией. Либо структура, либо геттеры/сеттеры.
• Коль скоро Writer хранит в себе ссылку, он некопируемый. Стоит это явно указать в сигнатуре, прописав соответствующие конструкторы и операторы.
• Node → HuffmanNode. Или HuffmanTreeNode. Или HuffmanTree::Node.

[Vasily Alferov]

+5 за тесты, +1 за архитектуру, +4 за стиль.

• Длинные флаги до сих пор не работают. На этот раз потому, что они могут идти в любом порядке. Например, --file <FILE> -c --output <OUTPUT>. Сделай, пожалуйста, нормально?

• Нормально -- это с исключениями в CLI. exit(1) -- это плохо.

• Название файла с тестами неправильное. А ещё твои тесты падают.

[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
test/TestHuffmanArchiver.cpp:4:
TEST CASE:  test HuffmanArchiver
  test compress

test/TestHuffmanArchiver.cpp:12: ERROR: CHECK( ar.get_size() == 1420 ) is NOT correct!
  values: CHECK( 4294967295 == 1420 )

test/TestHuffmanArchiver.cpp:13: ERROR: CHECK( ar.get_after_size() == 789 ) is NOT correct!
  values: CHECK( 0 == 789 )

test/TestHuffmanArchiver.cpp:14: ERROR: CHECK( ar.get_table_size() == 208 ) is NOT correct!
  values: CHECK( 4 == 208 )

0
0
4
===============================================================================
test/TestHuffmanArchiver.cpp:4:
TEST CASE:  test HuffmanArchiver
  test extract

test/TestHuffmanArchiver.cpp:23: ERROR: CHECK( ar.get_size() == 789 ) is NOT correct!
  values: CHECK( 0 == 789 )

test/TestHuffmanArchiver.cpp:24: ERROR: CHECK( ar.get_after_size() == 1420 ) is NOT correct!
  values: CHECK( 0 == 1420 )

test/TestHuffmanArchiver.cpp:25: ERROR: CHECK( ar.get_table_size() == 208 ) is NOT correct!
  values: CHECK( 4 == 208 )

===============================================================================
test/TestHuffmanArchiver.cpp:4:
TEST CASE:  test HuffmanArchiver
  test read_table

test/TestHuffmanArchiver.cpp:33: ERROR: CHECK( ar.get_table_size() == 204 ) is NOT correct!
  values: CHECK( 4 == 204 )

===============================================================================
[doctest] test cases:      3 |      2 passed |      1 failed |      0 skipped
[doctest] assertions:     22 |     15 passed |      7 failed |
[doctest] Status: FAILURE!

• За исключением того, что тесты падают, они почти ОК. Временные файлы для тестов -- зло. Юзай стримы.

[Vasily Alferov]

Топ!

Всё работает, всё проходит.
Баллы по максимуму, кроме архитектуры (4/5).

Хоть флаги теперь работают, но хотелось увидеть более расширяемое решение. То есть цикликом пройтись по флагам, достать все найденные значения и в конце проверить, что все нужные значения были и не противоречат друг другу.
Если будет необходимо дописать ещё один флаг, добавлять все перестановки в перебор как-то совсем грустно =(