hw 2 ostapenko.stepan

[ostapenko.stepan]

Поменял версию, т. к. пропарил первый дедлайн

[Дмитрий Лапшин (lapshin)]

Корректность: Почему-то иногда пишешь неправильные числа на экран, на ±1. 14.

Тесты:

1. Случайные тесты это не самое лучшее! Лучше бы крайние случаи поковырять. Но ты seed фиксируешь, это хорошо. 4.

Стиль:

1. Неймспейсы топ! Возможно не стоило так дробить, но сделано хорошо.
2. const size_t BITS_IN_CHAR = sizeof(char) * 8; смешно, потому что первое 1, а второе как раз содержательное. Есть стандартная константа.
3. ex_assert: а зачем эта функция в заголовке? Мне она вообще не нравится как функция: чище читается в месте вызова, а для потоков можно включить исключения.
4. Вообще не очень красиво с аргументами: сначала линию времени валидируем... И ничего не запоминаем, дальше каждую вещь ищем с нуля. Лучше уж сразу пройтись разобрать какая содержательная информация внутри, ругаться на ошибки, и потом, по-сути, гарантировать, что всё поняли, вот ответ (структуркой скажем).
5. Зачем отдельное исключение на out of bits я не понял, это же как eof.
6. Зачем у битовых читалок _указатель_ на поток? Ссылки же есть.
7. static const size_t BITS_IN_CHAR = 8 * sizeof(char); смешно. Тем более между файлами повторяется. И волшебные числа при этом остались.
8. Зачем всё на сырых указателях-то? Node * const * ааааа есть же контейнеры.
9. Комментарии странные. Давай не писать два предложения на одной строчке.
10. get_char_permutation имя какое-то непонятное.
11. Что за пробел в std::vector <char>?
12. in.rdbuf()->pubsetbuf(input_buf.get(), BUFFER_SIZE); а это-то зачем, стандартная буферизация не нравится?

Ууух. Тяжёлое впечатление: вроде всё по делу, но везде хватает шума. 4.

[ostapenko.stepan]

Корректность:

Почему-то иногда пишешь неправильные числа на экран, на ±1.

Странно-странно. Пока не знаю, что с этим делать.

UPD: исправил.

Тесты:

Попытался убрать случайные тесты там, где смог.

В TEST_SUITE("test HuffTree") рандом необходим, потому что иначе придется в каждом тесте руками вбивать 250 символов и 500 вершин, а это не очень круто (но у меня хотя бы seed зафиксирован).

В TEST_CASE("test 100 bad archives") и TEST_CASE("test archive undersize") рандом тоже необходим, т. к. я хочу убедиться, что на случайном файле у меня будет выброшен invalid_file_format (а в случае TEST_CASE("test archive undersize") еще и определенного вида invalid_file_format), но, опять же, здесь все seed'ы здесь зафиксированы. 100 запусков -- потому что хочется тщательно всё протестировать.

Стиль:

Что за пробел в std::vector <char>?

Тут я не понял, о чем вы.

Остальное исправил.

[ostapenko.stepan]

UPD:

Исправил вывод статистики на экран.

[Дмитрий Лапшин (lapshin)]

Что-то папка грязная.

Корректность: 15.

Тесты: 5.

Стиль:

1. Магические числа.
2. Пробелы между именем шаблона и параметрами (обычно не пишут).
3. Структуре с публичными полями геттеры/сеттеры не сильно нужны)
4. Сырые указатели на узлы дерева. При обходе можно достать ссылки!

7.

[ostapenko.stepan]

Папка не грязная. Там просто интеграционные тесты (постарался уменьшить их размер).

Стиль:

1. Убрал все найденные магические числа, кроме совсем уж странных случаев.
2. Кажется, вы говорили, что можно хоть как, главное -- делать всё в одном стиле (мне просто лень переделывать).
3. Добавил private )0)0) (На самом деле, сеттеры нужны, чтобы было удобнее выбрасывать исключения, а геттеры я решил оставить, чтобы не работать с std::optional где-то вне arg_utils.cpp).
4. Кажется, в данном случае сырые указатели удобнее (мб, конечно, это не так).

[Дмитрий Лапшин (lapshin)]

Ну странные стили цепляют глаза, а в рамках деталей действительно как угодно.

Всё таки сырые указатели которые ещё и паблик это такооое (указатель на корень дерева приватный, а дальше?). Указатели намекают на то что указуемый может смениться, а это при обходе не совсем так.

8.