HW #3

[Vasily Alferov]

=(

Корректность: 0
Тесты: 0 (отсутствуют)
Архитектура: 4
Стиль: 7

Общее замечание. Не компилится.

g++ -c -o obj/HuffmanArchiver.o -std=c++11 -Wall -Werror -Wextra -Iinclude  src/HuffmanArchiver.cpp
src/HuffmanArchiver.cpp: In member function ‘std::unordered_map<char, std::__cxx11::basic_string<char> > HuffmanArchiver::compress(HuffmanTree&)’:
src/HuffmanArchiver.cpp:43:21: error: moving a local object in a return statement prevents copy elision [-Werror=pessimizing-move]
   43 |     return std::move(encoded_bytes);
      |            ~~~~~~~~~^~~~~~~~~~~~~~~
src/HuffmanArchiver.cpp:43:21: note: remove ‘std::move’ call
src/HuffmanArchiver.cpp: In member function ‘std::unordered_map<std::__cxx11::basic_string<char>, char> HuffmanArchiver::extract(HuffmanTree&)’:
src/HuffmanArchiver.cpp:53:21: error: moving a local object in a return statement prevents copy elision [-Werror=pessimizing-move]
   53 |     return std::move(extracted_bytes);
      |            ~~~~~~~~~^~~~~~~~~~~~~~~~~
src/HuffmanArchiver.cpp:53:21: note: remove ‘std::move’ call
cc1plus: all warnings being treated as errors
make: *** [Makefile:21: obj/HuffmanArchiver.o] Error 1

А всё потому что не надо писать return std::move(x);. Ты таким образом заставляешь компилятор вызывать move constructor, в то время как copy elision позволяет не делать даже этого. ​Подробнее.

Ещё: убери, плиз, файлы .vscode/?

Замечания.

Корректность.
У меня не получилось получить не сегфолт даже на каких-то рандомных ручных тестах.
Кажется, что не работает совсем.

Архитектура.
В целом ОК.

• Ошибки в парсере на эксепшны заменить бы.
• Странно, что парсер делает действие. Логично было бы, чтобы у него спрашивали, что стоит сделать, а потом делали бы.

Стиль.

• Как минимум в конструкторе HuffmanTree, а может быть, ещё где-то, ссылки должны быть константными.
• Вспомогательные функции в cpp-шниках принято либо делать статическими, либо (что лучше) вкладывать в анонимные неймспейсы.
• code.push_back((byte >> position++ & 1) + '0') ;. Расставь скобки, пожалуйста, тут тебе не алгоритмы.

[Vasily Alferov]

Круто, практически божественно.

Корректность: 6/9
Тесты: 8/8
Архитектура: 5/5
Стиль: 7/8

• Корректность
  • Не работают длинные опции (--file, --output).
  • Выводимый размер сжатого файла почти всегда на 3 меньше его реального размера.
• Тесты
  • Тело compare_string_streams вроде можно заменить на CHECK(a.str() == b.str());.
• Архитектура ОК
• Стиль
  • Темплейтные методы надо реализовывать прямо в хедере. Или запихивать их как неметоды в анонимные неймспейсы. Тут скорее второе, потому что они тебе как методы не нужны. Как публичные точно. Причина: ты их просто не сможешь использовать из других файлов, точно также, как и темплейтные классы.

[gabitov.daniil]

All fixed

[Vasily Alferov]

Топ, фулл!