Change History (2)

comment:1 Changed 4 years ago by Дмитрий Свиридкин

Owner: changed from Дмитрий Свиридкин to zhemchuzhina.elizaveta
Type: ожидается проверкаожидаются исправления
/home/dmis/DATA/WORKSPACE/cpp-labs/hw_03/check/hw_03/src/CLI.cpp:36:1: warning: control reaches end of non-void function [-Wreturn-type]
   36 | }

Будет UB.

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

        std::ifstream ifs;
        std::ofstream ofs;
        ofs.exceptions(std::ofstream::failbit | std::ofstream::badbit);
        try {
            ifs.open(input_file.c_str(), std::ios::binary);
            ofs.open(output_file.c_str(), std::ios::binary);
        } catch (const std::ios_base::failure &e) {
            std::cerr << PLACE() << e.what();
        }

Почему бы не использовать конструктор *fstream? Не надо разрывать объявление и инициализацию.

Зафейлиться при открытии могут оба потока.

У конструкторов потоков есть перегрузка, принимающая string. Доставать сишную строку нет надобности.

    if (decode_flag) {
        std::ifstream ifs(input_file.c_str(), std::ios::binary);
        std::ofstream ofs(output_file.c_str());

бинырные файлы будут поломаны под Windows

На пустом файле:
Test failed -- invalid compressed size reported: 1 instead of 0
Файл действительно получился размера 0. Неверный вывод статистики.

alg.fill_codes_heap(codes_heap); почему бы этому методу не вернуть новую очередь, чем заполнять переданную? Возвращать одно значение более очевидное решщение, чем модифицировать единственный аргумент

    std::ifstream ifs("tmp.test");
    alg.set_frequency(ifs);
    alg.fill_codes_heap(code_heap);
            CHECK_EQ(code_heap.size(), 1);
            CHECK_EQ(code_heap.top().symbol, ';');
            CHECK_EQ(code_heap.top().freq, 4);

Дополнительный отступ перед каждым CHECK? Интересный стиль.

Строчка длиной в 262 символов в cmake... Лайфхак: делайте перенос строки

add_executable(EXE_NAME 
               src1
               src2
               ...)
#include <iostream>
#include <memory>
#include <fstream>
#include <algorithm>
#include "BitReader.h"
#include "BitWriter.h"
#include <queue>

Лучше группировать системные заголовки отдельно от ваших.

У вас все типы с больший буквы, но struct frequency

Архиватор лучше отделить от разархиватора. Первому нужна только таблица кодов. Второму только дерево.
Для таблицы кодов стоит завести свой класс/структуру и сделать конструктор для построения из дерева. Аналогично для дерева.

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

Этот метод публичный:
Huffman::HuffmanNode* Huffman::HuffmanTree::add_node(const std::vector<bool> &code)
А что будет, если добавят код, являющийся префиксом одного из существующих?
(например, архив испортился и какие-то биты побились?) Здесь никак не учитывается, что для корректности каждая новая добавленная цепочка должна заканчиваться листом дерева. И нельзя добавлять новые вершины к уже полученным ранее листьям.

Пляски с priority_queue точно не нужно делать публичными методами. Их надо инкапсулировать под простым интерфейсом: таблица частот на входе -> дерево на выходе.

Размещайте публичные поля и методы перед приватынми.
Приватыне кишки никому не нужны.


8 + 6 + 1.5 + 5

Last edited 4 years ago by Дмитрий Свиридкин (previous) (diff)

comment:2 Changed 4 years ago by Дмитрий Свиридкин

Resolution: задача сдана
Status: assignedclosed
Note: See TracTickets for help on using tickets.