-
Notifications
You must be signed in to change notification settings - Fork 33
Милов Виктор #31
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?
Милов Виктор #31
Conversation
Милов Виктор Николаевич(Попытка 2)
Милов Виктор Николаевич 2 неделя (2 попытка)
| while (i < size && array[i] == ' ') | ||
| { | ||
| i++; | ||
| spaces++; |
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.
префиксные инкрименты лучше использовать
| { | ||
| active_flags.push_back("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.
не оптимальное решение, создаем зачем-то вектор строк, с реалокациями памяти
| constexpr double FEET_TO_INCHES = 12.0; | ||
| constexpr double INCHES_TO_FEET = 1.0 / FEET_TO_INCHES; | ||
| constexpr double METERS_TO_CM = 100.0; | ||
| constexpr double CM_TO_METERS = 0.01; |
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.
не уверен что нужно столько констант, достаточно бы было 3, где-то можно было бы делить прям в выражениях. также желательно добавить их в безымянный namespace или пометить static
| constexpr double operator"" _cm_to_in(unsigned long long centimeters) | ||
| { | ||
| return static_cast<double>(centimeters) * CM_TO_METERS * METERS_TO_INCHES; | ||
| } 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 то зачем целочисленный вариант
| unsigned long long unsigned_value = static_cast<unsigned long long>(value); | ||
| unsigned_value &= mask; | ||
|
|
||
| // Выводим префикс |
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.
много лишних комментариев в коде
| mask = (1ULL << total_bits) - 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.
буквально это же написано строкой ниже, это излишний комментарий
| std::cout << "0b"; | ||
|
|
||
| // Выводим биты группами по 4 | ||
| for (int i = static_cast<int>(total_bits) - 1; i >= 0; 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.
префиксный декремент должен быть
| } | ||
| else | ||
| { | ||
| std::cout << "no solutions"; |
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.
лучше выделить особые случаи отдельно с && по условию, будет немного больше сравнениц на 0, но значительно читабельнее
| double root = -static_cast<double>(b) / (2.0 * a); | ||
| PrintRoot(root); | ||
| } | ||
| 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.
можно в ветви выше добавить return и убрать лишний уровень вложенности
| } | ||
|
|
||
| // Вычисление среднего значения квадратов и извлечение корня | ||
| double mean_of_squares = sum_of_squares / size; |
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
| @@ -1,6 +1,21 @@ | |||
| #include <stdexcept> | |||
|
|
|||
| double ApplyOperations(double a, double b, double (**operations)(double, double), size_t size) | |||
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 sum = 0.0; | ||
| for (size_t i = 0; i < size; ++i) | ||
| { | ||
| if (operations[i] != 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.
обычно принято проверять указатели без явного сравнения с nullptr, а просто пользуясь неявным приведением к булеву типу
| { | ||
| // throw std::runtime_error{"Not implemented"}; | ||
| // Проверка на некорректные указатели | ||
| if (begin == nullptr || end == 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.
!begin || !end было бы понятным
| if (begin == end) | ||
| { | ||
| return 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.
в данном случае когда все случаи возвращает одинаковое значение допустимообъединить, комментарии излишни, так как понятно по условию и названиям переменных, что происходит
| { | ||
| break; | ||
| } | ||
| 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.
префиксный декремент
| void PrintMemory(int value, bool reverse) | ||
| { | ||
| // throw std::runtime_error{"Not implemented"}; | ||
| unsigned char bytes[sizeof(int)]; |
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.
это лишнее, можно было обойтись без доп памяти
| void PrintMemory(double value) | ||
| { | ||
| PrintMemory(value, false); | ||
| } 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.
лишние функции, достаточно было использовать значениепо умолчанию для булева аргумента
| // throw std::runtime_error{"Not implemented"}; | ||
| unsigned char bytes[sizeof(double)]; | ||
| std::memcpy(bytes, &value, sizeof(double)); | ||
|
|
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.
далее идет полное дублирование кода, а нужно было написать третью функцию, которая в данном бы случае принимала массив (указатель), размер массива, флаг инверсии, и уже вызывалась из данных функций. Либо лучше было пойти по пути C++ и задействовать reinterpret_cast и писать сразу в поток
| { | ||
| // Big-endian | ||
| for (int i = sizeof(double) - 1; i >= 0; --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.
тела циклов полностью дублируются, можно переписать условия циклов, или вынести их в функцию, в общем в таких случаях желательно избавиться от дублирования
| @@ -1,6 +1,58 @@ | |||
| #include <stdexcept> | |||
|
|
|||
| const char *FindLongestSubsequence(const char *begin, const char *end, size_t &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.
программисты C++ предпочитают звездочку к типу приклеивать
| /* return_type */ FindLongestSubsequence(/* ptr_type */ begin, /* ptr_type */ end, /* type */ count) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| // Проверка на пустой диапазон | ||
| if (begin == 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.
это условие можно было добавить выше
| // Если текущий символ совпадает с символом в current_start | ||
| if (*current == *(current_start)) | ||
| { | ||
| current_length++; |
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 = begin; | ||
| reverse = true; | ||
| } | ||
| 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 | ||
| { | ||
| ++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.
это точно тернарный оператор должен быть
| for (size_t i = 0; i < count; ++i) | ||
| { | ||
| if (i > 0) | ||
| std::cout << ", "; |
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 << ", ...\n "; | ||
| elements_in_current_line = 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.
много кода, можно было компактнее и объединить некоторые условия
| const int *temp = ptr1; | ||
| ptr1 = ptr2; | ||
| ptr2 = temp; | ||
| } |
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 не должно происходить внутри функции и это необходимо показывать
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.
@lVictorl основные моменты по заданиям недель 1 и 2
- дублирование кода
- использование постфиксного инкремента вместо префиксного
- лишние комментарии
No description provided.