Conversation
01_week/tasks/addition/addition.cpp
Outdated
|
|
||
| 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.
Лишний каст, второй операнд не следует приводить явно, так как он бы преобразовался с помощью неявного каста. Принято использовать такую возможность и не писать явный каст самостоятельно второй раз
| } | ||
| size_t CharChanger(char array[], size_t, char delimiter = ' ') { | ||
| int counter = 0; // Счётчик повторяющихся символов | ||
| int write = 0; // Указатель для записи обработанного символа |
There was a problem hiding this comment.
В данном случае это позиция, не указатель, лучше не вводить в замешательство, а еще может лучше добавть префикс pos_write, тогда комментарии излишни
| throw std::runtime_error{"Not implemented"}; | ||
| } | ||
| size_t CharChanger(char array[], size_t, char delimiter = ' ') { | ||
| int counter = 0; // Счётчик повторяющихся символов |
There was a problem hiding this comment.
Лучше, чтобы название отражало то, что написанно в комментариях
|
|
||
| while (repeating_symbol != '\0'){ | ||
| if (repeating_symbol == array[read]){ | ||
| counter++; |
There was a problem hiding this comment.
здесь и далее корректней использовать префиксный инкремент
| while (repeating_symbol != '\0'){ | ||
| if (repeating_symbol == array[read]){ | ||
| counter++; | ||
| } else { |
There was a problem hiding this comment.
лучше для if сделать короткую ветвь с continue:
- Не придется листать много кода, что увидеть действие перед переходом на следующую итерацию цикла, либо даже лучше переписать на
for - Можно будет убрать
elseи лишний уровень вложенности
| } | ||
|
|
||
| if (counter >= 10){ | ||
| counter = 0; |
There was a problem hiding this comment.
как будто поусловию некорректно сбрасывать счетчик, тогда мы начнем считать заного и можно насчитать другое число
| return; | ||
| } | ||
|
|
||
| std::vector<std::string> set_flags; |
There was a problem hiding this comment.
Выглядит очень не оптимально, во первых вектор будет вызывать реалокацию при push_back.
Да и будем перекладывать строки из вектора в поток
| // Константы для преобразования | ||
| constexpr long double IN_TO_CM = 2.54L; | ||
| constexpr long double FT_TO_IN = 12.0L; | ||
| constexpr long double M_TO_CM = 100.0L; |
There was a problem hiding this comment.
Это правильный подход, добавить константы
| std::cout << 1; | ||
| } else { | ||
| std::cout << 0; | ||
| } |
There was a problem hiding this comment.
Можно 5 строк заменитьтернарным оператор выводить в поток по условию 1 или 0, будет компактнее
| } else { | ||
| double x = static_cast<double>(-c) / b; | ||
| std::cout << std::defaultfloat << std::setprecision(6) << x; | ||
| } |
There was a problem hiding this comment.
лучше не громоздить вложенность, особые случаи в отдельные ветви выделить, может немного больше будет сравнений, менее эффективно, но читаться будет приятней бнз такого уровня вложенности
| std::cout << std::defaultfloat << std::setprecision(6) << x; | ||
| } | ||
| } else { | ||
| long long discriminant = static_cast<long long>(b) * b - 4 * static_cast<long long>(a) * c; |
There was a problem hiding this comment.
Понятно для чего так сделано, но не лучше хранить дискриминант в double в данном случае
|
|
||
| double CalculateRMS(double values[], size_t size) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| if (size <= 0 || values == nullptr){ return 0.0; } |
There was a problem hiding this comment.
отсутствует пробел перед {
|
|
||
| double sum = 0; | ||
| for (size_t i=0; i < size; i++){ | ||
| sum+=std::pow(values[i], 2); |
There was a problem hiding this comment.
нет пробелов вокруг арифметических операторов и инициализации i
01_week/tasks/rms/rms.cpp
Outdated
| if (size <= 0 || values == nullptr){ return 0.0; } | ||
|
|
||
| double sum = 0; | ||
| for (size_t i=0; i < size; i++){ |
There was a problem hiding this comment.
Нужно использовать префиксный инкремент
|
|
||
| double ApplyOperations(double a, double b /* other arguments */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| double ApplyOperations(const double a, const double b, funcPtr* arr, size_t funcArraySize) { |
There was a problem hiding this comment.
наглядней было быиспользовать массив указателей на функцию, вместо указателя на указатель на функцию
| } | ||
|
|
||
| double cumulative_result = 0; | ||
| for (unsigned int i=0; i < funcArraySize; ++i){ |
| if (arr[i] == nullptr){ | ||
| continue; | ||
| } | ||
| cumulative_result+=arr[i](a, b); |
There was a problem hiding this comment.
пробелы вокруг оператора +=
02_week/tasks/last_of_us/README.md
Outdated
| указателя на начало и конец целочисленного массива, а также указатель на | ||
| функцию предикат, и возвращает указатель на найденный элемент. Если элемент | ||
| не найден, то возвращается указатель за последний элемент. | ||
| не найден, то возвращается указатель на последний элемент. |
There was a problem hiding this comment.
Это некорректное исправление так как, end не указывает на элемент и его нельзя разыменовывать
|
|
||
| if (last_element == nullptr){ | ||
| return end; | ||
| } |
There was a problem hiding this comment.
вероятно это лишняя проверка, так как можно last_element присвоить end изначально и тогда если успешных проверок не было, просто вернется last_element с нужным значением
| { | ||
| if (predicate(*begin)){ | ||
| last_element = begin; | ||
| } |
There was a problem hiding this comment.
можно упростить алгоритм поскольку нам нужен последний, быстрее идти с конца и при первом срабатывании сразу возвращать из функции нужный указатель
| << std::setfill('0') << std::setw(2) | ||
| << static_cast<unsigned int>(bytes[i]); | ||
| } | ||
| std::cout << std::dec << std::endl; |
There was a problem hiding this comment.
Это задание было конечно на reinterpret_cast, что позволило бы не создавать буфер копию текущего значения. Но даже так, зачем в данном случае полное дублирование?
Лучше было написать ещё одну функцию, которая бы принимала буффер, размер и флаг и вызывалась бы из данных функций
| void Swap(Stack& other); | ||
|
|
||
| bool operator==(const Stack& other) const; | ||
|
|
There was a problem hiding this comment.
много лишних пустых строк
04_week/tasks/stack/stack.cpp
Outdated
| return false; | ||
| } | ||
| } | ||
| return true; |
There was a problem hiding this comment.
можно вектора просто сравнить?
04_week/tasks/stack/stack.cpp
Outdated
| void Stack::Swap(Stack& other){ | ||
| Stack tmp = std::move(*this); | ||
| *this = std::move(other); | ||
| other = std::move(tmp); |
There was a problem hiding this comment.
std::swap для векторов
|
|
||
| buffer.resize(size, value); | ||
| capacity = size; | ||
| this->size = size; |
There was a problem hiding this comment.
использование this-> лучше избегать
| class RingBuffer { | ||
| std::vector<int> buffer; | ||
| size_t capacity = 0; | ||
| size_t size = 0; |
There was a problem hiding this comment.
лучше именовать внутренние переменные size_, m_size
18thday
left a comment
There was a problem hiding this comment.
@ilyamikhailov16 нужно исправлять:
- лучше использовать стандартные функции, вместо рукописного кода
- нужно использовать список инициализации
- много кода, не оптимальные методы
- неоптимальное сравнение Queue с безусловным копированием двух очередей
| }; | ||
|
|
||
|
|
||
| RingBuffer::RingBuffer(size_t size){ |
There was a problem hiding this comment.
корректнее использовать список инициализации
| capacity = size; | ||
| } | ||
|
|
||
| RingBuffer::RingBuffer(size_t size, const int& value){ |
There was a problem hiding this comment.
список инициализации и пробел перед {
| end = size-1; | ||
| } | ||
|
|
||
| RingBuffer::RingBuffer(const std::initializer_list<int>& list) : RingBuffer(list.size() > 0 ? list.size() : 1) |
There was a problem hiding this comment.
в данном случае также список инициализации бы помог
| ++begin; | ||
| } else if (end == begin && begin == capacity-1){ | ||
| begin = 0; | ||
| } |
There was a problem hiding this comment.
слишком много кода с излишним дублированием и лишним уровнем вложенности, лучше пересмотреть, что-то можно вынести в отдельные приватные методы
| buffer[end] = value; | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
можно было переиспользовать код TryPush в Push
04_week/tasks/queue/queue.cpp
Outdated
| } | ||
|
|
||
| bool Queue::operator==(const Queue& other) const { | ||
| return eqArrays(unionArrays(input, output), unionArrays(other.input, other.output)); |
There was a problem hiding this comment.
неоптимальное решение с копированием, зная устройство контейнера можно избежать дорогово копирования для сравнения. неоптимально также сначала копировать а потом сравнивать размеры
04_week/tasks/queue/queue.cpp
Outdated
| input = std::move(other.input); | ||
| output = std::move(other.output); | ||
| other.input = std::move(tmpInput); | ||
| other.output = std::move(tmpOutput); |
| return input.front(); | ||
| } else { | ||
| return output.back(); | ||
| } |
There was a problem hiding this comment.
тернарный оператор в одну строку
| output.push_back(input.back()); | ||
| input.pop_back(); | ||
| } | ||
| } |
There was a problem hiding this comment.
это может быть отдельный приватный метод с понятным именем для выполнимой операции
|
|
||
|
|
||
| Queue::Queue(std::vector<int> vector) : output(std::move(vector)) { | ||
| std::reverse(output.begin(), output.end()); |
There was a problem hiding this comment.
можно тогда складывать в input?
Refactor variable names for clarity in CharChanger function.
- Renamed counter variable for repeated symbols for clarity. - Replaced while loop with a for loop for better iteration over the character array. - Simplified logic for handling character transformations and counting repetitions.
- Removed unnecessary vector for storing flag names. - Streamlined output logic to directly print flags in a comma-separated format. - Improved readability by using a boolean flag to manage comma placement.
…nter - Changed initialization of last_element from nullptr to end to ensure proper return behavior. - Removed unnecessary null check before returning last_element.
- Changed the iteration to decrement the end pointer and return it directly when the predicate is satisfied. - Removed the last_element pointer initialization and unnecessary checks for clarity and efficiency.
…output logic - Replaced memcpy with direct reinterpretation of the number's bytes for both int and double types. - Streamlined the output logic to handle byte inversion directly during printing, enhancing readability and efficiency.
- Updated the logic for advancing the left pointer to improve clarity and efficiency. - Removed unnecessary while loop, directly assigning the right pointer to left for better readability.
…har pointers - Changed the return type of the existing FindLongestSubsequence function to const char* for better type safety. - Added an overload for FindLongestSubsequence that accepts non-const char pointers, utilizing the const version internally. - Improved pointer handling by removing unnecessary const_cast in the return statement.
- Updated input validation to simplify null and boundary checks. - Streamlined element printing logic by calculating total elements and using a single loop for both forward and backward traversal. - Improved output formatting by handling line breaks based on the specified count, enhancing readability.
…improved performance - Changed the parameter type of CalculateDataStats from std::vector<int> to const std::vector<int>& to avoid unnecessary copies and enhance efficiency.
- Replaced manual comparison logic with std::tie for equality, inequality, and relational operators in both Date and StudentInfo structs. - Streamlined operator implementations for improved readability and maintainability.
- Removed the use of a vector for storing flag names, directly printing flags in a comma-separated format. - Enhanced readability by utilizing a boolean flag to manage comma placement, improving the overall output formatting.
…ve performance - Added a reserve call for the result vector to optimize memory allocation based on the input vector size, enhancing efficiency during element insertion.
- Simplified the Swap method by utilizing std::swap for stack elements, enhancing performance. - Streamlined the equality operator implementation by directly comparing stack contents, improving code clarity.
- Simplified Push and TryPush methods by consolidating logic and reducing conditional checks. - Enhanced Pop and TryPop methods for better readability and streamlined operations. - Improved handling of buffer indices using modular arithmetic to simplify wrap-around logic.
…arity - Removed redundant array comparison methods and replaced them with a direct comparison of queue elements. - Implemented a helper function to access elements in a virtual vector, enhancing performance and memory usage. - Streamlined the equality operator to compare sizes and elements without additional memory allocation.
- Simplified the Swap method by replacing manual vector moves with std::swap, enhancing performance and code clarity.
No description provided.