-
Notifications
You must be signed in to change notification settings - Fork 33
Мирошниченко Евгений #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| } | ||
| avg = size_of_v > 0 ? avg/size_of_v : 0.0; | ||
| for (int num : vector){ | ||
| sko += (num-avg)*(num-avg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет пробелов
| for (int num : vector){ | ||
| sko += (num-avg)*(num-avg); | ||
| } | ||
| sko = size_of_v > 0 ? sqrt(sko / size_of_v) : 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::sqrt
| for (int num : vector){ | ||
| avg += num; | ||
| } | ||
| avg = size_of_v > 0 ? avg/size_of_v : 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет пробелов вокруг операторов
| char mark = 'A'; | ||
| int score = 0; | ||
| unsigned course = 1u; | ||
| Date birth_date{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в друго стиле написано, тоже через = тогда
| if (this->birth_date == other.birth_date){return false;} | ||
| else{ | ||
| return this->birth_date < other.birth_date; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Лишнее нагромождение с пустым условием,
elseтут не нужен - Можно сделать с помощью
std::tieв одну строку
| throw std::runtime_error{"Not implemented"}; | ||
| CheckFlags operator|(const CheckFlags& lhs, const CheckFlags& rhs) { | ||
| return static_cast<CheckFlags>( | ||
| (static_cast<uint8_t>(lhs) | static_cast<uint8_t>(rhs)) & 0x3F // Оставим только шесть бит |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это magic value, должно быть выражено через ALL (может быть даже вынесено в функцию с понятным именем для чего эта операция используется), иначе при изменении CheckFlags придется изменять все операторы, это очень плохо
| {CheckFlags::CERT, "CERT"}, | ||
| {CheckFlags::KEYS, "KEYS"}, | ||
| {CheckFlags::DEST, "DEST"} | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это очень сомнительное решение в операторе каждый раз выделять массив пар. неэффективно
| size_t index_good = 0; | ||
|
|
||
| for (size_t i = 0; i < vector.size(); ++i) { | ||
| if ((func) && (!func(vector[i]))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
если функция не передана, то странно вообще проходить по всему массиву, первая проверка должна быть вынесена вне цикла
| if ((func) && (!func(vector[i]))) { | ||
| continue; | ||
| } | ||
| else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else абсолютно не нужен, лишний уровень вложенности, так как в if есть continue
| size_t count = 0; | ||
| for (int value : vector) { | ||
| if (func(value)) ++count; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
очень странное решение дважды проходить по всему массиву и дважды делать проверку, вызывая предикат
|
|
||
| /* return_type */ MinMax(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| std::pair<std::vector<int>::const_iterator, std::vector<int>::const_iterator> MinMax(const std::vector<int>& vector) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
итератор требует псевдонима, поскольку дважды используется и возвращаемый тип очень длинный
| int x=0; | ||
| int y=0; | ||
|
|
||
| Coord2D() = default;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
две ;
| struct Circle { | ||
| Coord2D coord; | ||
| unsigned radius; | ||
| Coord2D coord{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
разный стиль инициализации переменных
| unsigned radius = 1u; | ||
|
|
||
| Circle() = default; | ||
| Circle(Coord2D c, unsigned r=1u): coord(c), radius(r){}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет пробела перед : и перед {
| auto min_iter = vector.begin(); | ||
| auto max_iter = vector.begin(); | ||
|
|
||
| if (vector.empty()) return {vector.end(), vector.end()}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
обычно не принято использовать сокращенную версию if
| else { | ||
| os << circle.coord << ", r = " << circle.radius << "]"; | ||
| } | ||
| return os; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
можно сильно сократить
убрать else и сделать 2 return, непосредственно из коротного условия и в конце (это нормально потому что обрабатывается особое условие)
поскольку оператор << возвращает ссылку на поток, лучше писать return os << ...
| } | ||
|
|
||
| std::ostream& operator << (std::ostream& os, CircleRegion circRegion){ | ||
| circRegion.second ? os << "+" : os << "-"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+ - - это символы, не строка, использовать строковый литерал в данном случае лишнее
возможно читать удобней
os << (circRegion.second) ? '+' : '-';
| std::ostream& operator << (std::ostream& os, CircleRegion circRegion){ | ||
| circRegion.second ? os << "+" : os << "-"; | ||
| os << circRegion.first; | ||
| return os; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
можно объединить со строкой выше
|
|
||
| /* return_type */ operator<<(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| std::ostream& operator << (std::ostream& os, const Coord2D& coordination){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в данном месте не принято окружать оператор пробелами,
|
|
||
| std::vector<int> Range(int from, int to, int step) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| std::vector<int> Range(int from, int to, int step=1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет пробелом вокруг оператора =
| result.resize(size_of_result); | ||
| size_t cur_idx = 0; | ||
|
|
||
| for (int i=from; step>0 ? i < to : i > to; i+=step){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет пробелов
|
|
||
| /* return_type */ Unique(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| std::vector<int> Unique(const std::vector <int>& vector) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
пробел перед шаблонным параметром не ставится
| while (iter != vector.end() && *iter == *cur_iter) { | ||
| ++iter; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не понимаю зачем проходить в два цикла
|
|
||
| if ((step == 0) || (from > to && step>0) || (from < to && step<0)) return result; | ||
|
|
||
| size_t size_of_result = step > 0 ? (to - from + step - 1) / step : (from - to - step - 1) / (-step); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
если вынести - то можно изменять по условию только +1 -1
| result.reserve(count); | ||
|
|
||
| for (auto iter = vector.begin(); iter != vector.end();){ | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
пустая строка
18thday
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EugeniusMiroshnichenko у меня следующие комментарии:
- бывают хорошие лаконичные решения, а бывает дикое дублирование или неэффективные решения (наталкивает на определенные мысли)
- не выдерживается единый стиль кода (где-то операторы выделены пробелами где-то все подряд написанно, иногда в одной строке разный стиль)
- излишне используется else и конструкции if-else
- неаккуратный код
No description provided.