Opened 3 years ago

Closed 3 years ago

#702 closed ожидается проверка (задача сдана)

HW#2

Reported by: movsin.marat Owned by: Святослав Власов
Component: HW #3 (Huffman) Version: 2.0
Keywords: Cc:

Description


Change History (4)

comment:1 Changed 3 years ago by Святослав Власов

Type: ожидается проверкаожидаются исправления

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

По стилю:

  1. std::map<char, TreeNode *> letters статический массив лучше подойдет, у тебя ведь фиксированный диапазон
  2. Bites -- укусы? :) Если ты хотел сказать "биты", то это Bits
  3. 	if(b){
    		return right_;
    	}
    	else{
    		return left_;
    	}
    

Тернарный же оператор есть, я много раз рассказывал: return b ? right_ : left_;
Аналогично operator< можно переписать

  1. int count3 = 5*sz+5 что это за Magic Numbers?
  2. Вектор пар в HuffmanArchiver::code/decode тоже проще заменить на статический массив
  3. count1/2/3 стоит переименовать во что-то более понятное

За корректность -- 5/10, тестов нет.

comment:2 Changed 3 years ago by Святослав Власов

UPD: 5/15

comment:3 Changed 3 years ago by movsin.marat

Type: ожидаются исправленияожидается проверка
Version: 1.02.0

comment:4 Changed 3 years ago by Святослав Власов

Resolution: задача сдана
Status: assignedclosed

Тесты прошли, кроме тех, что запускаются с измененным порядком аргументов

По стилю:

  1. Вот такое читать больно:
    TreeNode *n1 = new TreeNode(nodes.begin()->get_freq(), nodes.begin()->get_symb(), nodes.begin()->get_next(0), nodes.begin()->get_next(1));
    

Во-первых, можно переписать так:

auto node = nodes.begin();
TreeNode *n1 = new TreeNode(node->get_freq(), node->get_symb(), node->get_next(0), node->get_next(1));

А во-вторых, можно сделать просто конструктор от ноды и просто вызывать TreeNode *n1 = new TreeNode(*nodes.begin());

  1. Почему это делается не в конструкторе ноды? if(n1->get_next(0)!=nullptr)(n1->get_next(0))->set_parent(n1);
  2. std::vector<std::pair<char, int>> frequences; зачем вектор пар? Обычный std::array<int, 256> прекрасно бы выполнил эту задачу быстрее и удобнее. Не пришлось бы искать в цикле каждый символ в нем.
  3. Куча magic numbers осталась.
  4. Руками управляешь памятью. Про умные указатели было сказано уже не один раз.

В целом код очень сложно читать.

14/0/5

Note: See TracTickets for help on using tickets.