Дудин Лев #30
Дудин Лев #30
Conversation
This reverts commit 0a567ad.
| const int* FindLastElement(const int* begin, const int* end, bool predicate(int x)) | ||
| { | ||
| if (predicate && begin && end) | ||
| { |
There was a problem hiding this comment.
Лучше написать короткую ветвь, что делать делать если данные величины nullptr.
Что уберет уровень вложенности для основного решения
| if (predicate && begin && end) | ||
| { | ||
| size_t size = end - begin; | ||
| for (long i = size-1; i>=0; --i) |
There was a problem hiding this comment.
не хватает пробелов вокрыг - и >=.
также сомнительно использовать long, в него ппомещается меньше чем в size_t, что может првиести к проблемам
| { | ||
|
|
||
| int size = sizeof(int); | ||
| unsigned char *conv = reinterpret_cast<unsigned char *>(&num); // указатель на конкретный бит в инте |
There was a problem hiding this comment.
- указатель приклеиваем к типу, не к имени переменной
- некорректный комментарий, указатель на байт, не на бит
| unsigned char *conv = reinterpret_cast<unsigned char *>(&num); // указатель на конкретный бит в инте | ||
| std::string str; // строка, которую мы выведем | ||
|
|
||
| char dict[] = {"0123456789ABCDEF"}; // массив символов, в которые мы будем переводить |
There was a problem hiding this comment.
выглядит так будто мы решили задействовать много лишней памяти, для dict и для str
| str.push_back(dict[*conv >> 4]); | ||
| str.push_back(dict[*conv & 0b00001111]); | ||
| } | ||
| } |
There was a problem hiding this comment.
действия в цикле одинаковые, как будто можно было сделать сильно меньше 15 строк кода на данные действия, если переписать
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
лишние строки пустые
| } No newline at end of file | ||
| void PrintMemory(double num, bool is_inverted = false) | ||
| { | ||
|
|
There was a problem hiding this comment.
лишняя пустая строка
|
|
||
| std::cout << "0x" << str << std::endl; | ||
|
|
||
|
|
There was a problem hiding this comment.
лишние строки пустые
| str.push_back(dict[*conv >> 4]); | ||
| str.push_back(dict[*conv & 0b00001111]); | ||
| } | ||
| } |
There was a problem hiding this comment.
решение аналогичное и дублирует код предыдущей функции. Правильнее было общую часть вынести в третью функцию и после получения указателя на unsigned char*, зная sizeof от типа вызывать эту функцию с общим кодом из фукнции с double и с int
| char *FindLongestSubsequence(char *begin, char *end, size_t &count) | ||
| { | ||
| count = 0; | ||
| if (begin && end) |
There was a problem hiding this comment.
это должна быть проверка на особые условия и выход по короткой ветви с nullptr
| return nullptr; | ||
| } | ||
|
|
||
| const char *FindLongestSubsequence(const char *begin, const char *end, size_t &count) |
There was a problem hiding this comment.
полное дублирование кода в двух функциях, хотя используя const_cast можно вызывать данную функцию из другой
| a = b; | ||
| b = c; | ||
| } | ||
|
|
There was a problem hiding this comment.
лишние функции в решении, можно было обойтись без такого количества функций и без шаблонов
| { | ||
| std::cout << '['; | ||
|
|
||
| size_t len = abs(begin-end); |
There was a problem hiding this comment.
отсутствуют пробелы
| else | ||
| { | ||
| std::cout<<"[]\n"; | ||
| } |
There was a problem hiding this comment.
опять зачем-то лишний уровень вложенности и короткое условие идет после основного
| T *current = begin - i; | ||
| current == end + 1 ? (std::cout << *current) : (std::cout << *current << ", "); | ||
| } | ||
| std::cout<<"]\n"; |
There was a problem hiding this comment.
отсутствуют пробелы вокруг оператора
| if (limiter && i != 0 && !(i % limiter)) | ||
| std::cout << "...\n" << ' '; | ||
|
|
||
| i!=(len-1)? ( std::cout << begin[i] << ", "): ( std::cout << begin[i] << "]\n"); |
There was a problem hiding this comment.
отсутствуют пробелы с двух сторон от операторов, лишние пробелы после (
| std::cout << "...\n" << ' '; | ||
| T *current = begin - i; | ||
| current == end + 1 ? (std::cout << *current) : (std::cout << *current << ", "); | ||
| } |
There was a problem hiding this comment.
дублирование кода,можно было записать компактнее
| double second_num= 0.; | ||
|
|
||
| for (size_t i=0; i< data.size(); ++i){ | ||
| output.avg+=data[i]; |
| for (size_t i=0; i< data.size(); ++i){ | ||
| output.avg+=data[i]; | ||
| first_num += static_cast<double>(data[i])*static_cast<double>(data[i]); | ||
| second_num+= data[i]; |
|
|
||
| for (size_t i=0; i< data.size(); ++i){ | ||
| output.avg+=data[i]; | ||
| first_num += static_cast<double>(data[i])*static_cast<double>(data[i]); |
There was a problem hiding this comment.
лишний каст, достаточно одного каста
| #include <stdexcept> | ||
|
|
||
| #include <vector> | ||
| #include <math.h> |
There was a problem hiding this comment.
#include <cmath> подключаем библиотеку на C++
| output.avg /= data.size(); | ||
| output.sd = first_num - 2. * second_num * output.avg + output.avg * output.avg * data.size(); | ||
| output.sd /= data.size(); | ||
| output.sd = sqrt(output.sd); |
There was a problem hiding this comment.
для библиотек C++ используем std::sqrt
| std::tie(s1.mark, s2.score, s1.course, s2.birth_date); | ||
| } | ||
|
|
||
| bool operator!=(const StudentInfo &s1, const StudentInfo &s2) { return !(s1 == s2); } |
There was a problem hiding this comment.
а что тут стиль кода сломался, чтобы было на новой строке?
| throw std::runtime_error{"Not implemented"}; | ||
| CheckFlags operator|(CheckFlags f, CheckFlags s) | ||
| { | ||
| return static_cast<CheckFlags>(((static_cast<u_int8_t>(f) & 0x3F) | (static_cast<u_int8_t>(s) & 0x3F))); |
There was a problem hiding this comment.
0x3F - это magic value который не используется в коде, тем более в данном случаее есть ALL
| #include <sstream> | ||
|
|
||
| enum class CheckFlags : uint8_t { | ||
| enum class CheckFlags : u_int8_t |
There was a problem hiding this comment.
А это что за ужас? u_int8_t принято использовать uint8_t
| return false; | ||
|
|
||
| // Проверяем пересечение флагов | ||
| return ((f_val & s_val) ==f_val)||((f_val & s_val)==s_val); |
There was a problem hiding this comment.
нет пробелов вокруг операторов, середина формулы очень плохо из-за этого читается
| { | ||
| u_int8_t value = static_cast<u_int8_t>(flags) & 0x3F; | ||
|
|
||
| if (value > 0b00111111 || value == static_cast<u_int8_t>(CheckFlags::NONE)) |
There was a problem hiding this comment.
magic value сделают код не универсальным при изменении перечисления
| out << ", "; | ||
| first = false; | ||
| out << "DEST"; | ||
| } |
There was a problem hiding this comment.
не самое оптимальное решение по количеству кода
| if (func(data[i])) | ||
| { | ||
| if (i!= counter){ | ||
| data[counter] = data[i]; |
There was a problem hiding this comment.
слишком большой уровень вложенности, принято сначала отрабатывать по коротким ветвям
| { | ||
| if (func(data[i])) | ||
| { | ||
| if (i!= counter){ |
There was a problem hiding this comment.
отсутствует пробел перед != и {
| throw std::runtime_error{"Not implemented"}; | ||
| void Filter(std::vector<int> &data, bool (*func)(int)) | ||
| { | ||
| if (!data.empty() && func != nullptr) |
There was a problem hiding this comment.
можно просто проверить !func обычно так принято для указателей, не писать явно nullptr
| { | ||
| std::vector<size_t> found; | ||
| if (func && !data.empty()) | ||
| { |
| #include <stack> | ||
|
|
||
| class Queue | ||
| { |
There was a problem hiding this comment.
реализацию нужно было вынести вне класса
| auto it = num.begin(); | ||
| for (; it < num.end(); ++it) | ||
| { | ||
| this->Push(*it); |
There was a problem hiding this comment.
this-> не нужно использовать в данном случае
|
|
||
|
|
||
| std::vector<int> *in = new std::vector<int>; | ||
| std::vector<int> *out = new std::vector<int>; |
There was a problem hiding this comment.
вектор сам работает с динамической памятью, его как раз писали для того, чтобы в ручную это не делать
|
|
||
| // Простое поэлементное сравнение | ||
| Queue copy1 = *this; | ||
| Queue copy2 = other; |
There was a problem hiding this comment.
не оптимально, зная внутреннее устройство не должно быть копирования
| #include <vector> | ||
| #include <stack> | ||
|
|
||
| class Queue |
There was a problem hiding this comment.
некорректная реализация с использованием динамической памяти под объект, который самостотельно управляет динамической памятью
2 participants
No description provided.