Conversation
| int64_t a_64 = static_cast<int64_t>(a); | ||
| int64_t b_64 = static_cast<int64_t>(b); | ||
| int64_t sum = a_64 + b_64; | ||
| return sum; |
There was a problem hiding this comment.
Три лишних переменных. принято сразу записывать выражение в строке return. Причем достаточно бы было тогда одного static_cast, второй операнд не следует приводить явно, так как он бы преобразовался с помощью неявного каста. Принято использовать такую возможность и не писать явный каст самостоятельно второй раз
| if (islower(current)) { //проверка на пропись | ||
| array[position] = current - 32; | ||
| } | ||
| if (isupper(current)) { //проверка на пропись |
There was a problem hiding this comment.
вот поэтому такие комментарии лишние , потому что из-за копипасты только запутывают, к тому же "проверка на пропись" абсолютно ничего не говорит о том что происходит
| if (!first) check_flags += ","; | ||
| check_flags += "DEST"; | ||
| first = false; | ||
| } |
There was a problem hiding this comment.
дублирование кода, много строк с одинаковыми действиями, можно было избавиться от них
| } | ||
| constexpr long double operator""_cm_to_m(long double cm) { | ||
| return cm/100.0; | ||
| } |
There was a problem hiding this comment.
Не принято использовать magic values в коде, пожалуй кроме 100, 1000 к примеру.
В данном случае корректнее добавить константы с понятными именами, например для 12 0.0254 0.3048 и уже в коде их использовать
|
|
||
| ++comma_position; | ||
| } | ||
| std::cout << bit_num; |
There was a problem hiding this comment.
судя по коду можно было писать сразу в поток без создания дополнительного массива
| @@ -0,0 +1,41 @@ | |||
| // ft -> в другие единицы | |||
| constexpr long double operator""_ft_to_in(long double ft) { | |||
| return ft*12.0; | |||
There was a problem hiding this comment.
не хватает пробелов вокруг операторов
| return; | ||
| } | ||
| //Проверяем дискрименант | ||
| double D = b*b-4*a*c; |
There was a problem hiding this comment.
- чтобы избежать проблем с переполнением при умножении может быть лучше вычислять в double 4.0 и явный каст одного из
b - пробелы вокруг операторов ставятся
| std::cout << std::setprecision(6) << x1; | ||
| return; | ||
| } | ||
| if (D < 0){ |
There was a problem hiding this comment.
лишнее условие, можно сразу выполнить код, а лучше поменять местами с D > 0, так как это более короткая ветвь
| } | ||
| //Проверяем дискрименант | ||
| double D = b*b-4*a*c; | ||
| if (D > 0){ |
There was a problem hiding this comment.
нет пробела перед {
и лучше поменять местами с более короткой ветвью
| double CalculateRMS(double values[], size_t size) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| } No newline at end of file | ||
| if (!values || size == 0){ |
| for (size_t i = 0; i < size; ++i){ | ||
| sum_square += std::pow(values[i], 2); | ||
| } | ||
| double RMS = std::sqrt(sum_square/size); |
There was a problem hiding this comment.
- лишняя перемменная
- пробелы вокруг оператора
| throw std::runtime_error{"Not implemented"}; | ||
| } No newline at end of file | ||
| double ApplyOperations(double a, double b, double (*func[])(double, double), size_t size) { | ||
| if (size == 0){ |
| throw std::runtime_error{"Not implemented"}; | ||
| } No newline at end of file | ||
| const int* FindLastElement(const int* begin, const int* end, bool (*predicate) (const int)) { | ||
| if ((end - begin) <= 0 || end == nullptr || begin == nullptr){ |
There was a problem hiding this comment.
проверку указателей можно сокращать, используя неявное приведение к bool !end || !begin
| return end; | ||
| } | ||
| const int* last = end; | ||
| for (; begin < end; ++begin){ |
| std::cout << std::hex << std::uppercase << std::setw(2) << std::setfill('0') << static_cast<int>(ptr_char[i]); | ||
| } | ||
| std::cout << std::endl; | ||
| } |
There was a problem hiding this comment.
полное дублирование кода, после получения указателя на символ и зная sizeof от типа, можно было вызвать функцию, содержащую все инструкции для вывода, тем самым избавиться от дублирования
| std::cout << std::hex << std::uppercase << std::setw(2) << std::setfill('0') << static_cast<int>(ptr_char[i]); | ||
| } | ||
| std::cout << std::endl; | ||
| } |
There was a problem hiding this comment.
выглядит как достаточно большое дублирование, можно было бы переписать так, чтобы не дублирвоать
| } | ||
|
|
||
| count = max_length; | ||
| return const_cast<char*>(longest_start); |
There was a problem hiding this comment.
Это UB. если изначально был указатель на константу, то снимать константность это UB.
Хорошим решением было бы две функции одна принимающая и возвращающая указатели на константы, а вторая без константности. Тогда из неконстантной версии можно было бы вызвать константную и после снять константность с помощью const_cast
| int dist = end - begin; | ||
| unsigned int count = 0; | ||
| if (begin == nullptr || end == nullptr || (dist*dist) == 0){ // Проверка на nullptr или пустой массив | ||
| numbers+="]"; |
There was a problem hiding this comment.
пробелы вокруг операторов улучшают читаемость
| std::string numbers = "["; | ||
| int dist = end - begin; | ||
| unsigned int count = 0; | ||
| if (begin == nullptr || end == nullptr || (dist*dist) == 0){ // Проверка на nullptr или пустой массив |
There was a problem hiding this comment.
комментарий излишний, не совсем понятна цель dist возводить в квадрат
| while (true){ | ||
| int num = *begin; | ||
| std::string reverse_number = ""; | ||
| while (num / 10 != 0) { // // Конвертация int в str и запись в массив numbers |
There was a problem hiding this comment.
тут нет массива numbers, комментарий не соответствует действительности
| dist = end - begin; | ||
| } | ||
| numbers += "]"; | ||
| std::cout << numbers << std::endl; |
There was a problem hiding this comment.
можно было обойтись без лишних действий с добавлением и инверсией строки
| auto* temp = ptr1; | ||
| ptr1 = ptr2; | ||
| ptr2 = temp; | ||
| } |
| size_t N = data.size(); | ||
| for (size_t i = 0; i < N; ++i){ | ||
| sum_of_x += data[i]; | ||
| sum_of_squares_x += static_cast<double>(data[i])*data[i]; |
There was a problem hiding this comment.
пробелы вокруг оператора
| if (student1.score != student2.score) return student1.score < student2.score; | ||
| if (student1.course != student2.course) return student1.course > student2.course; | ||
| if (student1.birth_date != student2.birth_date) return student1.birth_date < student2.birth_date; | ||
| return false; |
There was a problem hiding this comment.
в данном случае можно было также использовать std::tie за счет расположения различных аргументов student1 и student2 в одном операнде, можно выразить как < так и >
std::tie(rhs.mark, lhs.score, ....) < std::tie(lhs.mark, rhs.score, ....)
- lhs rhs компактнее и более индормативно student1, student2, на крайний случай student_lhs или lhs_student
| /* return_type */ operator|(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| CheckFlags operator|(CheckFlags flags1, CheckFlags flags2) { | ||
| uint8_t all = static_cast<uint8_t>(CheckFlags::ALL); |
There was a problem hiding this comment.
думаю эта переменная излишня, можно выполнять данную операцию по месту
| CheckFlags operator|(CheckFlags flags1, CheckFlags flags2) { | ||
| uint8_t all = static_cast<uint8_t>(CheckFlags::ALL); | ||
| uint8_t value1 = static_cast<uint8_t>(flags1) & all; // Обрезаем все биты слева которые заходят за допустимый размер all | ||
| uint8_t value2 = static_cast<uint8_t>(flags2) & all; |
There was a problem hiding this comment.
выглядит как три лишних переменных
|
|
||
| /* return_type */ operator|(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| CheckFlags operator|(CheckFlags flags1, CheckFlags flags2) { |
There was a problem hiding this comment.
lhs, rhs - и компактней и понятней
| if (!first) os << ", "; | ||
| os << "DEST"; | ||
| first = false; | ||
| } |
There was a problem hiding this comment.
как будто однотипный код много раз повторяется
|
|
||
| /* 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>& vec) { |
There was a problem hiding this comment.
было бы неплохо задействовать псевдоним для std::vector::const_iterator, так как тип выглядит громоздко.
- пробел между
std::pairи<лишний
| int max = vec[0]; | ||
| auto min_it = vec.begin(); | ||
| auto max_it = vec.begin(); | ||
| for (auto it = vec.begin(); it!= vec.end(); ++it){ |
There was a problem hiding this comment.
пробелы перед {, здесь и далее
| throw std::runtime_error{"Not implemented"}; | ||
| std::vector<int> Range(int from, int to, int step = 1) { | ||
| std::vector<int> result; | ||
| if ((to - from)*step <= 0) return result; // проверка на невалидные параметры |
There was a problem hiding this comment.
пробелы вокруг операторов
| ++write_index; | ||
| } | ||
| } | ||
| ++write_index; // Костыль чтобы учесть первый добавленный элемент |
There was a problem hiding this comment.
это локальная переменная, зачем?
| for (size_t i = 1; i < vec.size(); ++i) { | ||
| if (result[write_index] != vec[i]){ // если следующий элемент не равен предыдущему то добавляем его в result | ||
| result.push_back(vec[i]); | ||
| ++write_index; |
There was a problem hiding this comment.
может тогда стоило в условии result[write_index++]
18thday
left a comment
There was a problem hiding this comment.
@MelnikovGYu основные моменты:
- magic values
- UB const_cast
- дублирование кода, которое можно избежать
| for (size_t i = 0; i < stack1.size(); ++i){ | ||
| if (stack1[i] != stack2[i]) return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
зачем копировать ели можно сравнить вектора напрямую через равенсво?
There was a problem hiding this comment.
@MelnikovGYu лишнее копирование в операторе равенства Stack, мало заданий
No description provided.