Conversation
WIP: week_01
WIP: week 02
| if(ptr == end) { break; } | ||
| ++count; | ||
| } | ||
| std::cout << "]\n"; |
There was a problem hiding this comment.
В данном случае это тоже практически полное дублирование кода, можно и нужно от него избавиться
| template <typename T> | ||
| void SwapPtr(T*& ptr1, T*& ptr2) { | ||
| // Проверяем, указывают ли ptr1 и ptr2 на указатели | ||
| if constexpr (std::is_pointer_v<std::remove_pointer_t<T>>) { |
There was a problem hiding this comment.
как будто это уже лишнее, и вывод типов шаблона справился бы без этого
|
|
||
| void SwapPtr(/* write arguments here */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| template <typename T> |
There was a problem hiding this comment.
можно было и без шаблона получить решение
| stats.sd += static_cast<long long>(num) * num; | ||
| } | ||
| // Вычисляем среднее значение | ||
| stats.avg = static_cast<double>(stats.avg) / numbers.size(); |
There was a problem hiding this comment.
stats.avg уже имеет тип double поэтому каст тут лишний и можно было использовать /=
| } | ||
| else { | ||
| stats.sd = 0; | ||
| } |
There was a problem hiding this comment.
как будто это лишнее, либо можно было компактнее переписать
| static const std::map<char, int> priority = { | ||
| {'Z', 0}, {'D', 1}, {'C', 2}, {'B', 3}, {'A', 4} | ||
| }; | ||
| return priority.at(mark); |
There was a problem hiding this comment.
это плохой вариант, так как символы оценок уже упорядочены, иможно было использовать напрямую коды символов, данные подход не гибкий, в случае добавления буков например Y или F, любой, придется переписыватть все начения, очень неудобно
| if (lhs.course != rhs.course) { | ||
| return lhs.course > rhs.course; | ||
| } | ||
| return lhs.birth_date < rhs.birth_date; |
There was a problem hiding this comment.
это тоже можно сделать компактно с std::tie так как в если менять местами поля для rhs lhs в правом и левом операнде, при одном знаке <, можно некоторые поля сравнивать на <, а некотоыре на >
|
|
||
| for (size_t i = 0; i < vec.size(); ++i) { | ||
| if (predicate(vec[i])) { | ||
| if (i != write_index) { |
There was a problem hiding this comment.
лучше выше убрать лишнюю вложенность либо инвертировав условие и continue сделать у предиката
| #include <stdexcept> | ||
| #include <vector> | ||
|
|
||
| using IntVecIterator = std::vector<int>::const_iterator; |
There was a problem hiding this comment.
лучше отметить что он константный, cit, как вариант можно было сделать два псевдонима, чтобыбыло понятнее какой min какой max, тут понятно по названию функции, но в CitMin CitMax например тоже можно
| } | ||
|
|
||
| if (*local_min < *min_it) min_it = local_min; | ||
| if (*local_max >= *max_it) max_it = local_max; |
There was a problem hiding this comment.
как будто можно было сразу стремиться делать только нужную операцию, объединив условия, не используя локальные итераторы, меньше памяти и кода
| struct Coord2D { | ||
| int x; | ||
| int y; | ||
| Coord2D(int x_val = 0, int y_val = 0) : x(x_val), y(y_val) {} |
There was a problem hiding this comment.
лучше было указать значения по умолчанию у полей и не писатьтривиальный конструктор для структуры, так как компилятор самостоятельно с этим справится
| unsigned radius = 1; | ||
| Circle() : coord(), radius(1) {} // Default | ||
| explicit Circle(Coord2D c, unsigned r = 1) : coord(c), radius(r) {} | ||
| Circle(unsigned r) : coord(), radius(r) {} |
There was a problem hiding this comment.
аналогичная история, конструкторы лишние, если использовать тот же подход
| } | ||
|
|
||
| std::ostream& operator<<(std::ostream& os, const CircleRegionList& circle_vector) { | ||
| os << '{'; |
There was a problem hiding this comment.
лучше обработать отдельно случай empty короткой ветвью с выходом
и тем самым вынести основной код извложенности в if, когда тело длинное, приходится посмотреть, чем все заканчивается, чтобы понять что в итоге будет выводится, а при коротких ветвях с return все понятно
| sequence.push_back(current); | ||
| current += step; | ||
| } | ||
| } |
There was a problem hiding this comment.
возможно может стоит предпосчитать количество шагов и оставить один цикл, чтобы не дублировать
| } | ||
| std::vector<int> Unique(std::vector<int> sorted_nums) { | ||
| std::vector<int> unique_nums; | ||
| if(sorted_nums.empty()) {return unique_nums;} |
There was a problem hiding this comment.
что- то тут стилькода поменялся
| int previous = *(sorted_nums.begin()); | ||
| unique_nums.push_back(previous); | ||
| for(auto num : sorted_nums) { | ||
| if(previous != num) { |
There was a problem hiding this comment.
лучше убрать вложенность previous == num - > continue
There was a problem hiding this comment.
@WhoAmI1909 отметил следующие моменты
- дублирование кода
- UB const_cast
- местами лишние переменные
- не все задачи решены
2 participants
No description provided.