Плотников Денис #20
Плотников Денис #20
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.
Лишний каст, второй операнд кастовать не обязателен, так как произойдет неявный каст к int64_t, и принято этим пользоваться, оставляя код чище
| } | ||
|
|
||
| size_t src {}; // индекс чтения | ||
| size_t dst {}; // индекс записи |
There was a problem hiding this comment.
пробел обычо не ставят перед {} в данном случае
| size_t dst {}; // индекс записи | ||
|
|
||
| while (array[src] != '\0' && src < size) { | ||
| char c { array[src++] }; // текущий символ |
There was a problem hiding this comment.
аналогично предыдущему и для переменных и списков инийиализации обычно пробелами не обрамляют фигурные скобки с внутренней стороны
| // Подсчет количества одинаковых символов | ||
| size_t count {1}; | ||
| while (array[src] != '\0' && src < size) { | ||
| const char next { array[src] }; |
There was a problem hiding this comment.
лучше не именовать переменную используемую в единственном месте, а использовать по месту, тем более на следующем цикле она уже не next, а next_next
| array[dst++] = '0'; | ||
| } else { | ||
| array[dst++] = static_cast<char>('0' + count); | ||
| } |
There was a problem hiding this comment.
в данном случае просится тернарный оператор в одну строку
| c = '*'; | ||
| } else if (std::islower(static_cast<unsigned char>(c))) { | ||
| c = std::toupper(static_cast<unsigned char>(c)); | ||
| } else if (std::isupper(static_cast<unsigned char>(c))) { |
There was a problem hiding this comment.
как будто лучше вместо else опставить условие !std::isupper(), а данную проверку убрать
| case 3: out += "CERT"; break; | ||
| case 4: out += "KEYS"; break; | ||
| case 5: out += "DEST"; break; | ||
| } |
There was a problem hiding this comment.
Это круто, понравилось решение, единственное нет отступа у case
| void PrintCheckFlags(CheckFlags flags) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| void PrintCheckFlags(CheckFlags flags) { | ||
| constexpr static const auto all { static_cast<uint8_t>(CheckFlags::ALL) }; |
There was a problem hiding this comment.
Считаю это излишним, так как применяется в единственном месте, и переменная мало весит, чтобы её делать static, не уверен что будет эффекктивнее чем ходить недалеко по стеку, вероятно лучше по месту применить каст
|
|
||
| std::string out {'['}; | ||
| bool first { true }; | ||
| for (uint8_t i = 0; std::cmp_less(i, std::popcount(all)); ++i) { |
There was a problem hiding this comment.
на каждом цикле считать std::popcount(all) не целесообразно
There was a problem hiding this comment.
Если all помечено constexpr и std::popcount --- тоже constexpr, то разве будет выполняться пересчёт на каждой итерации?
В таком случае и статичность all кажется вполне уместной.
| // Метры → дюймы | ||
| constexpr double operator""_m_to_in(long double v) { | ||
| return static_cast<double>(v / 0.0254); | ||
| } |
There was a problem hiding this comment.
лучше в коде избегать magic value (за исключением 100 наверное), неплохо бы завести безымянный namespace с понятными константами 2.54, 0.3048, 12 и их использовать
|
|
||
| void PrintBits(long long value, size_t bytes) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| using value_t = decltype(value); |
There was a problem hiding this comment.
для использования в одном месте немного сомнительно
|
|
||
| std::string out { "0b" }; | ||
| for (size_t i = 0; i < bits; ++i) { | ||
| const value_t mask { (static_cast<value_t>(1) << (bits - 1 - i)) }; |
There was a problem hiding this comment.
можно использовать литерал для long long будет более выразительно нагромождения с псевдонимом
| std::cout << "infinite solutions"; | ||
| } else { | ||
| std::cout << "no solutions"; | ||
| } |
There was a problem hiding this comment.
в данно случае можно и тернарный вместо 5 строк использовать
| if (a == 0) { | ||
| if (b == 0) { | ||
| if (c == 0) { | ||
| std::cout << "infinite solutions"; |
There was a problem hiding this comment.
может проверок меньше но вложенность не так выразительна, как разбить на отдельные случаи
| std::cout << -static_cast<double>(c) / b; | ||
| } | ||
| } else { | ||
| const int d { b * b - 4 * a * c }; |
There was a problem hiding this comment.
D тогда уж, но по-хорошему нужно не пожалеть символов и написатьдискриминант
|
|
||
| for (size_t i = 0; i < operations_count; ++i) { | ||
| if (operations[i] == nullptr) continue; | ||
| sum += operations[i](std::as_const(a), std::as_const(b)); |
There was a problem hiding this comment.
не совсем понятно зачем нужна обертка, если функция принимает по значению
| const std::size_t j { n - 1 - i }; | ||
| std::cout << std::setw(2) << static_cast<unsigned int>(p[j]); | ||
| } | ||
| } |
There was a problem hiding this comment.
судя по коду достаточно применить тернарный оператор для получения индекса и избежать дублирования кода циклов
| void PrintMemory(const T value, const bool invert = INVERT_DEFAULT) | ||
| { | ||
| const byte_view_t p { reinterpret_cast<byte_view_t>(addressof(value)) }; | ||
| const size_t n { sizeof(value) }; |
There was a problem hiding this comment.
в большинстве слчаев sizeof() вычислиться на этапе компиляции, поэтому можно не класть переменную на стек и везде в коде использовать sizeof(value)
| #include <cstddef> // size_t | ||
| #include <iterator> // std::next, std::prev | ||
|
|
||
| char* FindLongestSubsequence(const char* cbegin, const char* cend, size_t& count) { |
There was a problem hiding this comment.
В данно случае сигнатура неверная, так как вызов от константной функции UB, возвращать неконстантный указатель нельзя из такой функции, должно быть две версии перегруженные по константности и там где известно что вызвана была не константная версия можно использовать константную и вернуть не константный указатель, используя const_cast, но не наоборот
| CheckCurrentSequence(); | ||
|
|
||
| //? Тест MutableString на момент написания кода предполагает, | ||
| //? что возвращается неконстантный указатель |
There was a problem hiding this comment.
Подразумевает две версии функции
| #include <iterator> // std::next, std::prev | ||
|
|
||
| char* FindLongestSubsequence(const char* cbegin, const char* cend, size_t& count) { | ||
| if (cbegin == nullptr || cend == nullptr || cbegin >= cend) { |
There was a problem hiding this comment.
для указателей можно использовать !cbegin || !cend (явное сравнение с nullptr обычно реже используется)
|
|
||
| if (num < total) { | ||
| if (limit != NO_LIMIT && num % limit == 0) | ||
| { |
| auto PrintElement = [&](const size_t idx, const int* const p) { | ||
| std::cout << *p; | ||
|
|
||
| const auto num { idx + 1 }; |
There was a problem hiding this comment.
может тогда имеет смысл передавать idx_next? idx не используется и зачем-то создается копия элемента увеличенная на 1
| for (size_t i = 0; p < cend; ++p, ++i) { | ||
| PrintElement(i, p); | ||
| } | ||
| } |
There was a problem hiding this comment.
применение лямбды сомнительно с учетом того что код дублируется, как будто необходимо просто имзенять --p ++p в тернарном операторе и использовать один цикл и не использовать лямбду с захватом вовсе, так как кода мало и используется в одном месте
| bestLen = currLen; | ||
| bestStart = currStart; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Несколько сомнительно использовать лямбду в коде
| #include <cstddef> | ||
|
|
||
| static constexpr size_t NO_LIMIT = 0; | ||
| static constexpr const size_t LIMIT_DEAFULT {NO_LIMIT}; |
| void SwapPtr(/* write arguments here */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| template <typename T> | ||
| void SwapPtr(T*& pp1, T*& pp2) { |
There was a problem hiding this comment.
это все-таки ссылка на указатель а не указатель на указатель
|
|
||
| const size_t n { data.size() }; | ||
|
|
||
| //? Математически корректно использовать n-1 для несмещённой оценки дисперсии выборки, |
There was a problem hiding this comment.
Дело не в корректности, а в наличие смещенной и несмещенной оценок. По умолчанию в данном случае подразумевается смещенная, так как не требует какой-то доп. квалификации и непонимания почему тесты не проходят
| if (birth_date != other.birth_date) | ||
| return birth_date < other.birth_date; | ||
|
|
||
| return false; |
There was a problem hiding this comment.
таким образом можно все свести к одной строке используя std::tie
|
|
||
| /* return_type */ operator^(/* args */) { | ||
| throw std::runtime_error{"Not implemented"}; | ||
| auto operator&(const CheckFlags lhs, const CheckFlags rhs) { |
There was a problem hiding this comment.
Почему тогда не принимать неконстантную копию? и далее не плодить ещё переменных
| if (circle_region.second) { | ||
| // внутренняя область | ||
| os << "+"; | ||
| } else { |
| } else { | ||
| // внешняя область | ||
| os << "-"; | ||
| } |
| if (step == 0 || from == to || | ||
| (step > 0 && from > to) || | ||
| (step < 0 && from < to)) | ||
| { |
| const size_t size { | ||
| (to - from) % step == 0 | ||
| ? static_cast<size_t>((to - from) / step) | ||
| : static_cast<size_t>((to - from) / step) + 1 |
There was a problem hiding this comment.
судя по выражению тернарный оператор можно использовать только для + 1
2 participants
No description provided.