Тутынина Полина #35
Тутынина Полина #35
Conversation
Исправлена опечатка
| case 'A': return 5; | ||
| default: return 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
это излишне, так как сам порядковый номер буквы возрастающий и мы всегда может её перевести в число без написания отдельной функции, причем при добавлении одной оценки, например F, пришлось перенумеровать все значения, не универсально
|
|
||
| // 4. Сравниваем дату рождения (более старый < более молодой) | ||
| // Более ранняя дата (старее) должна быть "меньше" | ||
| return lhs.birth_date < rhs.birth_date; |
There was a problem hiding this comment.
можно было короче с std::tie если в левом операнде и правом поменять для какого-то праметра lhs rhs, то поменяется знак сравнения на противоположный, хотя между std::tie будет <
| throw std::runtime_error{"Not implemented"}; | ||
| // Вспомогательная функция для фильтрации бит, | ||
| // биты 0-5 установлены, биты 6-7 нули | ||
| constexpr CheckFlags sanitize(CheckFlags flags) { |
There was a problem hiding this comment.
лучше, чтоюы она было только для данной единицы трансляции, например посредством безымянного namespace
| uint8_t original = static_cast<uint8_t>(flags); | ||
|
|
||
| // Инвертируем и оставляем только 0-5 биты | ||
| uint8_t inverted = (~original) & mask; |
There was a problem hiding this comment.
можно было конечно прям по месту это сделать
| } | ||
|
|
||
| // Собираем названия активных флагов | ||
| std::vector<std::string> active_flags; |
There was a problem hiding this comment.
реалокации для вектора, и как будто для данной задачи немного накладно каждый раз создавать вектор строк добавлять туда, а потом пробегаться по нему при выводе и копировать в поток
| else if (*current > *max_it) { | ||
| max_it = current; | ||
| } | ||
| else if (*current == *max_it) { |
There was a problem hiding this comment.
получается это тоже самое, что в условии выше сделать >=
|
|
||
| Circle() = default; | ||
|
|
||
| Circle(Coord2D center, unsigned r = 1) : coord(center), radius(r) {} |
There was a problem hiding this comment.
тривиальный конструктор, излишне для структуры
| throw std::runtime_error{"Not implemented"}; | ||
| // Обработка некорректных параметров | ||
| if (step == 0) { | ||
| return std::vector<int>(); //пустой вектор |
There was a problem hiding this comment.
можно {} так как тип мы уже писали в возвращаемом значении
| // Если шаг отрицательный, но from <= to - диапазон пуст | ||
| if (step < 0 && from <= to) { | ||
| return std::vector<int>(); | ||
| } |
There was a problem hiding this comment.
как будто можно объединить условие step == 0 и условие по тернарному оператору, например для step < 0
| if (step > 0) { // для возрастающего диапазона | ||
| count = (to - from + step - 1) / step; | ||
| } else { // для убывающего диапазона | ||
| count = (to - from + step + 1) / step; |
There was a problem hiding this comment.
один тернарный оператор -1 или +1 сразу при инициализации count
| for (int i = from; i > to; i += step) { | ||
| result.push_back(i); | ||
| } | ||
| } |
There was a problem hiding this comment.
раз мы уже занем количество элементов, то можно подумать и переписать без if на один цикл for
| double real_; | ||
| double imag_; | ||
|
|
||
| static constexpr double PI = 3.14159265358979323846; |
There was a problem hiding this comment.
в C++20 есть numbers, так же есть M_PI, в общем самостоятельно лучше не писать значение
| static constexpr double PI = 3.14159265358979323846; | ||
|
|
||
| // Нормализация фазы в диапазон (-π, π] | ||
| static double normalize_angle(double angle) { |
There was a problem hiding this comment.
хотелось бы конечно увидеть объявления все, а определения вне класса
| } | ||
|
|
||
| // Помещаем в input_stack в обратном порядке для сохранения | ||
| // или в output_stack с правильным порядком |
There was a problem hiding this comment.
может тогдане стоило организовывать два цикла с обходом по всем элементам и временный вектор, а сразу заполнить подходящий стек? или применить идиому copy and swap для временного вектора и подходящего
| // Метод Front (константная версия) : доступ только на чтение к первому элементу | ||
| // Возвращает константную ссылку на первый элемент очереди | ||
| // Не модифицирует объект | ||
| const int& Queue::Front() const { |
There was a problem hiding this comment.
может константная и не константная версия должны быть консистентными? нарушается принцип наименьшего удивления, когда один метод то копирует много элементов то работает быстро и сразу отдает ссылку
| return input_stack.back(); | ||
| } else { | ||
| return output_stack.front(); | ||
| } |
There was a problem hiding this comment.
return с тернарным оператором в одну строку
|
|
||
| // Создаем копии для безопасного сравнения | ||
| Queue copy1 = *this; | ||
| Queue copy2 = other; |
There was a problem hiding this comment.
как быдто это очень дорого. мы же знаем внутреннее устройство, почему не пройтись по очередям в нужном порядке и не сравнить элементы?
| size_t head; // индекс самого старого элемента | ||
| size_t tail; // индекс, куда будет добавлен следующий элемент | ||
| size_t count; // текущее количество элементов | ||
| bool is_full; // флаг полного заполнения буфера |
There was a problem hiding this comment.
если поддерживать data с нужным capacity и имея count, флаг поддерживать лишнее
| std::vector<int> Vector() const; | ||
| }; | ||
|
|
||
| // Конструктор от вместимости буфера |
| is_full = false; | ||
| } | ||
|
|
||
| // Конструктор от вместимости буфера и начального значения |
| is_full = true; | ||
| } | ||
|
|
||
| // Конструктор от std::initializer_list |
| // Копирующий конструктор | ||
| RingBuffer::RingBuffer(const RingBuffer& other) | ||
| : data(other.data), head(other.head), tail(other.tail), | ||
| count(other.count), is_full(other.is_full) {} |
There was a problem hiding this comment.
дефолтный получается?
| if (is_full) { | ||
| // Перезаписываем самый старый элемент | ||
| data[tail] = value; | ||
| tail = next_pos(tail); |
There was a problem hiding this comment.
выглядит так будто это общая часть, которую можно вынести
| tail = next_pos(tail); | ||
| ++count; | ||
| is_full = (count == data.size()); | ||
| } |
There was a problem hiding this comment.
либо как вариант задействовать TryPush а если не получилось то заполнить как для полного, строк тоже будет меньше
| int& RingBuffer::Front() { | ||
| if (Empty()) { | ||
| static int dummy; | ||
| return dummy; |
There was a problem hiding this comment.
такого требования не было, все-таки ждали UB, но с таким подходом как будто лучше сделать один общий dummy
|
|
||
| // Возвращает линейное представление буфера (от старого к новому) | ||
| std::vector<int> RingBuffer::Vector() const { | ||
| std::vector<int> result(count); |
There was a problem hiding this comment.
а почему мы не делаем reserve и push_back?
| } | ||
| head = 0; | ||
| tail = count % new_capacity; | ||
| is_full = (count == new_capacity); |
There was a problem hiding this comment.
в таких случаях лучше сделать move и return
| head = 0; | ||
| tail = count % new_capacity; | ||
| is_full = (count == new_capacity); | ||
| } else { |
There was a problem hiding this comment.
а здесь убрать уровень вложенности
| @@ -0,0 +1,16 @@ | |||
| # CMAKE generated file: DO NOT EDIT! | |||
There was a problem hiding this comment.
обычно всю папку build*/ добавляют в .gitignore файл, и не пушат их, поскольку это все локальные сборки для конкретной машины
There was a problem hiding this comment.
@Polina785643 отметил следующие моменты
- хороший код
по задачам 1-3: - дублирование кода
- избыточные комментарии
- префиксный инкремент
по задачам 4: - неконсистентность констатного и не константного метода
- есть лишние копирования
- комментариев больше кода
2 participants
No description provided.