HW #3 Golovin Valery

[golovin.valeriy]

Пока не сделал тесты, но вроде все работает

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

src/HuffmanArchiver.cpp:12:34:   required from here
/usr/include/c++/9/bits/stl_tree.h:780:8: error: static assertion failed: comparison object must be invocable as const
  780 |        is_invocable_v<const _Compare&, const _Key&, const _Key&>,

На пустом файле и файле из одного символа: Failed to decompress archive: exit code 233
И на не пустом тоже.

Получайте ресурсы в конструкторах и ссвобождайте их в деструкторах, а не вызывайте явные close и прочие.

Пишите тесты.

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

Добавлен лог ошибок.

---

Помещайте публичные члены классов в начале описания, а не в конце.
Приватные кишки никому не нужны.

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

/usr/include/c++/9/bits/stl_tree.h:780:8: error: static assertion failed: comparison object must be invocable as const
  780 |        is_invocable_v<const _Compare&, const _Key&, const _Key&>,
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/hw_03.dir/build.make:122: CMakeFiles/hw_03.dir/src/HuffmanArc

/home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/include/ByteFileReader.hpp: In member function ‘ByteFileReader& ByteFileReader::operator>>(T&)’:
/home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/include/ByteFileReader.hpp:24:5: warning: no return statement in function returning non-void [-Wreturn-type]

---

==130506== Invalid write of size 1
==130506==    at 0x14F9A7: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Invalid read of size 1
==130506==    at 0x11A06C: std::char_traits<char>::length(char const*) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x11BACE: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x120E84: parseArgs(int, char const**) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x14F9D4: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Invalid write of size 1
==130506==    at 0x14FD4C: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Invalid read of size 1
==130506==    at 0x11A06C: std::char_traits<char>::length(char const*) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x11BACE: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x120E84: parseArgs(int, char const**) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x14FD6A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Invalid read of size 1
==130506==    at 0x11A06C: std::char_traits<char>::length(char const*) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x11BACE: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x120E84: parseArgs(int, char const**) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x150099: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Invalid read of size 1
==130506==    at 0x11A06C: std::char_traits<char>::length(char const*) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x11BACE: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x120E84: parseArgs(int, char const**) (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1500EC: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce9fa is 0 bytes after a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 
==130506== Mismatched free() / delete / delete []
==130506==    at 0x483D1CF: operator delete(void*, unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x1501BF: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==  Address 0x4dce860 is 0 bytes inside a block of size 10 alloc'd
==130506==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==130506==    by 0x14F91A: _DOCTEST_ANON_FUNC_2() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x1347EE: doctest::Context::run() (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506==    by 0x13520B: main (in /home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/test_hw_03)
==130506== 

---

Статистика выводится некорректно

Running test  valgrind input/empty.0.core
Test failed -- invalid compressed size reported: 72 instead of 9
Test failed -- invalid compressed size reported: 72 instead of 9

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

Разархивирование не работает с приложенным файлом.

Проблемы со статистикой можете воспроизвести, порпоюовав сжать свой CMakeLists.txt

set(CMAKE_CXX_STANDARD 20)
Эко вы лихо. Еще ни один компилятор это поддерживает по-нормальному.

---

Как-то иногда работает (после того, как пришлось пофиксить UB)

---

    } catch (const std::invalid_argument& err) {
        std::cerr << err.what();
        return 0;
    }

Как так 0, ошибка же?!

 [[nodiscard]] std::unordered_map<uint8_t, std::queue<bool>>& getMap();

std::queue для представления строки кода?! Это самое неожиданное решение!

std::pair<uint64_t, uint64_t> importBinary(BitFileReader& begin);

Это публичный метод, а что он возвращает? std::pair --- самый неудачный выбор для того, чтобы выразить что-то осмысленное.
Почему бы этому методу не быть статическим, возвращающим прочитанную таблицу?

public:
    void initTabelle(BitFileReader& begin);
    void extract(BitFileReader& begin, BitFileReader end, BitFileWriter& dest);
    void extractFile(const std::string& in_file, const std::string& out_file);

Почему первые два метода публичные? Как ими пользоваться, чтоб ничего не сломать?

Почему метод build у Nodе, а не у дерева? Узел не должен уметь строить деревья.

template <class Iterator>
class HuffmanTree 

Почему итератор параметризует весь класс, а не только методы, которые импользуют итераторы?

class HuffmanTree {
public:
    void build(std::unordered_map<uint8_t, std::size_t>& frequency_tabelle);
    void build(std::unordered_map<uint8_t, std::queue<bool>>& code_tabelle);

Почему бы этим не заниматься конструктору?

---

4 + 6 (краевые случаи не протестили -> программа не работает на них) + 4 + 7

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

• И в третий раз

rebind<_Key>::other>::const_iterator = std::_Rb_tree_const_iterator<std::shared_ptr<HuffmanNode> >; std::set<_Key, _Compare, _Alloc>::value_type = std::shared_ptr<HuffmanNode>]’
/home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/src/HuffmanTree.cpp:39:75:   required from here
/usr/include/c++/9/bits/stl_tree.h:780:8: error: static assertion failed: comparison object must be invocable as const
  780 |        is_invocable_v<const _Compare&, const _Key&, const _Key&>,

• Статистика сжатия/разжатия выводится с ошибкой в 1 байт.

  Test failed -- invalid compressed size reported: 11019 instead of 11018
  Test failed -- invalid compressed size reported: 11019 instead of 11018
  

• Вложенность кода в main можно уменьшить с помощью function-try-block

•         huffman.compressFile(args.in_name_, args.out_name_);
  
          std::cout << huffman.getSizeInputFile() << '\n';
          std::cout << huffman.getSizeOutputFile() << '\n';
          std::cout << huffman.getSizeTabelle() << '\n';
  

Почему бы методу compressFile не вернуть эту статистику? Зачем ее хранить в качестве состояния

•     ByteFileReader in_iterator(in_file);
      ByteFileReader Eof;
  

Раз этот класс имеет семантику итератора, наверное, его стоило назвать XXXXXIteraror

*

void HuffmanTree::buildBranch(HuffmanNode& node, std::queue<bool>& code, uint8_t symbol) {

В этом методе стоит кинуть исключение, если код внезапно получился непрефиксный (архив побился)

*

    std::shared_ptr<HuffmanNode> a(new HuffmanNode(12, 'a')), b(new HuffmanNode(1232, 'b')),
        c(new HuffmanNode(a, b)), d(new HuffmanNode(0, 'd'));

Лучше make_shared.

*

    //str_c выдает всегда один укзатель
    //argv[1] = std::string("-f").c_str();
    //argv[2] = std::string("InputFile").c_str();
    //argv[3] = std::string("-c").c_str();
    //argv[4] = std::string("-o").c_str();
    //argv[5] = std::string("OutputFile").c_str();

Вот так делать не надо, потому что указатели будут висячими. После каждой ; строка умирает.

•  const char* argv[7];
      for (auto& i : argv) i = new char[10];  // я не смог придумать, как сделать без new
  

std::vector<std::string> args = { .... };
std::vector<const char*> argv;
std::transform(begin(args), end(args), 
               back_inserter(argv), 
               [](const auto& arg){
                  return arg.c_str();
               });

strcpy((char*)argv[1], "--file");

1. const_cast
2. не надо так делать

•     std::ofstream("tmp");
      {
          BitFileWriter out("tmp");
  

У вас одновременно существуют два объекта пишуших в один файл, никак между собой не синхронизированных. Как-то не хорошо.

Лучше BitFileWriter? обернуть не файл, а std::ostream. И в тестах писать в (o)stringstream

---

8.5 + 7.5 + 4.5 + 8