Жамбакиев Радий #34
Жамбакиев Радий #34
Conversation
2 week two
|
|
||
| if (step > 0 && from > to) { | ||
| return range; | ||
| } |
There was a problem hiding this comment.
можно 3 условия не писать
03_week/tasks/unique/unique.cpp
Outdated
|
|
||
| uniqueElem.reserve(v.size()); | ||
|
|
||
|
|
There was a problem hiding this comment.
лишние пустые строки
04_week/tasks/stack/stack.cpp
Outdated
| void Swap(Stack& other); | ||
|
|
||
| bool operator==(const Stack& other) const { | ||
| return this->stack_ == other.stack_; |
There was a problem hiding this comment.
this-> в данном случае и обычно лишнее
04_week/tasks/queue/queue.cpp
Outdated
| } | ||
| }; | ||
|
|
||
| Queue(std::vector<int> v) : outputV_(v.rbegin(), v.rend()){}; |
There was a problem hiding this comment.
лишнее копирование, принимается копия вектора
04_week/tasks/queue/queue.cpp
Outdated
| outputV_.reserve(size); | ||
| }; | ||
|
|
||
| Queue(const int size) { |
There was a problem hiding this comment.
достаточно одной версии size_t
04_week/tasks/queue/queue.cpp
Outdated
|
|
||
| Queue(std::initializer_list<int> list) : inputV_(list) {}; | ||
|
|
||
| Queue(const size_t size) { |
There was a problem hiding this comment.
const излишне в таком контексте, поскольку принимаем копию и это конструктор класса, никто нам не мешает умножить size на два например и создать больший контейнер
04_week/tasks/queue/queue.cpp
Outdated
| outputV_.reserve(size); | ||
| }; | ||
|
|
||
| inline void Push(int val); |
There was a problem hiding this comment.
Ключевое слово inline применяется сейчас в другом контексте. В данном случае оно избыточно
04_week/tasks/queue/queue.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| return this->GetQueueAsVector() == other.GetQueueAsVector(); |
There was a problem hiding this comment.
- this лишний
- создание полных копий для сравнения двух очередей, очень ресурсоемко данном случае
| } | ||
|
|
||
| std::vector<int> Queue::GetQueueAsVector() const { | ||
| std::vector<int> result; |
There was a problem hiding this comment.
размер известен заранее, можно избежать реалокаций
|
|
||
| outputV_.pop_back(); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
много лишних пустых строк
| return outputV_.back(); | ||
| } | ||
|
|
||
| const int& Queue::Front() const { |
There was a problem hiding this comment.
константная и неконстантная версия не консистентны, реализованы по разному, в одной из них используется полное копирование
|
|
||
| private: | ||
| std::vector<int> ringBuff_{}; | ||
| size_t head_ = 0; |
There was a problem hiding this comment.
лишний пробел перед =
|
|
||
| RingBuffer(size_t capacity) : ringBuff_(capacity > 0 ? capacity : 1), head_(0), tail_(0), size_(0) {} | ||
|
|
||
| RingBuffer(size_t capacity, const int iVal) : ringBuff_(capacity > 0 ? capacity : 1, iVal), head_(capacity), tail_(0), |
There was a problem hiding this comment.
лишний const в аргументах
| RingBuffer(size_t capacity) : ringBuff_(capacity > 0 ? capacity : 1), head_(0), tail_(0), size_(0) {} | ||
|
|
||
| RingBuffer(size_t capacity, const int iVal) : ringBuff_(capacity > 0 ? capacity : 1, iVal), head_(capacity), tail_(0), | ||
| size_(capacity > 0 ? capacity : 1) {} |
There was a problem hiding this comment.
поехало выравнивание
| ++size_; | ||
| } else { | ||
| // аналогично head | ||
| tail_ = (tail_ + 1) % ringBuff_.capacity(); |
There was a problem hiding this comment.
где-то ringBuff_.capacity(), где-то cap лучше приести к единообразию
| } | ||
|
|
||
| bool RingBuffer::TryPop(int& val) { | ||
| if (size_ > 0) { |
There was a problem hiding this comment.
лучше под if короткую ветвь использовать
| ringBuff_[i] = tempV[i]; | ||
| } | ||
|
|
||
| ringBuff_.shrink_to_fit(); |
There was a problem hiding this comment.
много лишнего в данном методе, лучше применить идиому copy and swap,
непонятно почему нужно резервировать временный вектор не под новый размер, а под минимальный, поему сразу не располагаются элементы нужным образом, а сначала добавляются в конец, а потом реверсивный порядок создается.
и зачем вызывается shrink_to_fit() после resize()?
| v.reserve(size_); | ||
|
|
||
| for (size_t i = 0; i < size_; ++i) { | ||
| v.push_back(ringBuff_[(tail_ + i) % ringBuff_.size()]); |
There was a problem hiding this comment.
ringBuff_.size() - корректнее все-таки capacity(), как минимум надо привести к единообразию
| } | ||
|
|
||
| std::vector<int> RingBuffer::Vector() const { | ||
| std::vector<int> v; |
There was a problem hiding this comment.
имена лучше длинее одной буквы
04_week/tasks/phasor/phasor.cpp
Outdated
| constexpr double EPS_ = 1e-12; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
лишняя пустая строка
04_week/tasks/phasor/phasor.cpp
Outdated
| } | ||
|
|
||
| double RadToDeg(double rad){ | ||
| return rad * 180 / M_PI; |
There was a problem hiding this comment.
надо к единообразию привести, 180.0 и 180
04_week/tasks/phasor/phasor.cpp
Outdated
| class Phasor { | ||
| public: | ||
| Phasor() = default; | ||
| Phasor(double A, double rad) { |
04_week/tasks/phasor/phasor.cpp
Outdated
| } | ||
|
|
||
| // Сложение коммутативно | ||
| Phasor operator+(double rhs, const Phasor& lhs) { |
There was a problem hiding this comment.
слева все-таки lhs справа rhs, можно было просто выразить через предыдущий оператор
04_week/tasks/phasor/phasor.cpp
Outdated
| return (lhs + (-rhs)); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
лишняя пустая строка
18thday
left a comment
18thday
left a comment
There was a problem hiding this comment.
@notron124 отметил следующие моменты:
- много оптимальных решений
- хорошо читаемый, аккуратный код
- внутри класса обычно не используют для вызова методов
this-> - в задачах классы встречаются лишние копирования, не понравился Resize() в кольцевом буфере
2 participants
No description provided.