-
Notifications
You must be signed in to change notification settings - Fork 33
Коптелов Константин #39
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
|
|
||
| int64_t Addition(int a, int b) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| return static_cast<int64_t>(a) + static_cast<int64_t>(b); |
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 new_char; | ||
|
|
||
| // Преобразуем символ по правилам | ||
| if (c >= '0' && c <= '9') { |
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.
есть готовые функции для данных проверок, например isdigit, лучше их использовать, так как понятное имя и стандартная функция лучше рукописного кода
| next_new_char = delimiter; | ||
| } else { | ||
| next_new_char = '_'; | ||
| } |
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 (next_new_char == new_char) { | ||
| 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.
в таких местах должен быть префиксный инкремент, а постфиксный только там где нужна копия
| if (next_new_char == new_char) { | ||
| count++; | ||
| next++; | ||
| } 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.
это короткая ветвь корректней сделать if (next_new_char != new_char) break и убрать else
| } | ||
|
|
||
| // Начинаем вывод | ||
| printf("["); |
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.
printf это не C++
| if (!first) printf(","); | ||
| printf("DEST"); | ||
| first = false; | ||
| } |
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.
в данном случае это тоже дублирование, много однотипных действий с разными аргументами
| } | ||
|
|
||
| constexpr double operator"" _m_to_cm(long double meters) { | ||
| return static_cast<double>(meters * 100.0L); |
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 не принято в коде, удобнее сделать константы с понятными именами и уже их использовать в коде, безымянный namespace для их объединения как вариант использвоания
| printf("1"); | ||
| } else { | ||
| printf("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.
это должен быть тернарный оператор
| // b*x + c = 0 → x = -c/b | ||
| double x = -static_cast<double>(c) / b; | ||
| std::cout << x << std::endl; | ||
| } |
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 (discriminant < 0) { | ||
| // Отрицательный дискриминант - нет действительных корней | ||
| std::cout << "no solutions" << std::endl; |
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, поскольку не придется листать оставшийся код, чтобы понять, что больше ничего не добавляется, а далее везде убраны else,
|
|
||
| // Корень из среднего значения квадратов | ||
| return std::sqrt(mean_of_squares); | ||
| } No newline at end of file |
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.
комментарии тут излишне, по именам и коду понятно что происходит, лишнего не нужно писать
| double mean_of_squares = sum_of_squares / size; | ||
|
|
||
| // Корень из среднего значения квадратов | ||
| return std::sqrt(mean_of_squares); |
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.
mean_of_squares используется только в одном месте, заводить переменную излишне, можно подставить выражение в сразу в функцию квадратного корня
| if (!reverseBytes) { | ||
| // Little-endian порядок (по умолчанию) | ||
| std::cout << "0x"; | ||
| for (int i = 0; i < static_cast<int>(sizeof(temp)); 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.
префиксный инкремент
| unsigned char byte = (temp >> (i * 8)) & 0xFF; | ||
| std::cout << std::hex << std::setw(2) << std::setfill('0') | ||
| << static_cast<unsigned int>(byte); | ||
| } |
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::cout << std::hex << std::setw(2) << std::setfill('0') | ||
| << static_cast<unsigned int>(byte); | ||
| } | ||
| } |
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.
полное дублирвоание тела, нужно было после memcpy вызывать функцию выполняющую работу, передавая например указатель или сам temp и sizeof от типа
| // Проверка на пустой диапазон | ||
| if (begin == end) { | ||
| count = 0; | ||
| return nullptr; |
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.
действие условий одинаковое, нужно было объединять так как пустой диапазон тоже можно считать некорректным
| current_start = current; | ||
| current_length = 1; | ||
| } | ||
| current++; |
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.
префиксный инкремент должен быть + если ++current выполняется всегда, может стоило переписать на for
| if (begin == end) { | ||
| std::cout << "[]\n"; | ||
| return; | ||
| } |
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.
стоит объединить условия
| last--; | ||
| if (last <= end) break; | ||
| } | ||
| } |
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 можно было написать компактнее
|
|
||
| void SwapPtr(/* write arguments here */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| void SwapPtr(const int*& a, const int*& b) { |
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** temp = a; | ||
| a = b; | ||
| b = temp; | ||
| } No newline at end of file |
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.
@winnermrco-boop основные моменты:
- мало решено заданий
- дублирование кода
- нет использования стандартных функций првоерки диапазонов символов в CharChanger
No description provided.