Conversation
This reverts commit 5ba2b46.
| bool operator!=(const Stack& otherStack) const; | ||
|
|
||
|
|
||
| private: |
There was a problem hiding this comment.
лучше убрать дополнительный отступ у модификаторов доступа и сооветственно один уровень отступа у методов и полей
| std::vector<int> buffer_; | ||
| size_t oldestElement_ = 0; | ||
| size_t indexNextElem_ = 0; | ||
| size_t currentCountElem_ = 0; |
There was a problem hiding this comment.
несколько громоздкие названия, данное можно заменить на size_
| buffer_.resize(capacity); | ||
| oldestElement_ = 0; | ||
| indexNextElem_ = 0; | ||
| currentCountElem_ = 0; |
There was a problem hiding this comment.
это должно быть все в списке инициализации
| buffer_[0] = initialValue; | ||
| indexNextElem_ = 1 % buffer_.capacity(); | ||
| currentCountElem_ = 1; | ||
| } |
There was a problem hiding this comment.
это плохо, можно было в списке инициализации вызвать соответствуеющийконструктор вектора, а тут достаточно объемный код, но дае еслитак, лучшеизбавится от else и лишней вложенности и поставить внтруи if return
| oldestElement_ = 0; | ||
| indexNextElem_ = 0; | ||
| currentCountElem_ = 0; | ||
| for (int value : init) Push(value); //Добавил range based, так как будто бы уютнее смотрится |
There was a problem hiding this comment.
аналогично, можно вызвать конструктор вектора от init
| indexNextElem_ = NextIndex(indexNextElem_); | ||
| } else { | ||
| buffer_[indexNextElem_] = value; | ||
| indexNextElem_ = NextIndex(indexNextElem_); |
There was a problem hiding this comment.
в каждой из ветвей есть общий код, дублирование
| for (size_t i = 0; i < otherQueue.inputDat.size(); ++i) { | ||
| temp2.push_back(otherQueue.inputDat[i]); | ||
| } | ||
| return temp1 == temp2; |
There was a problem hiding this comment.
зная устройство контейнера, лучше избежать копирования при сравнении, также реализовано с дублированием кода
| tempOutput.push_back(tempInput.back()); | ||
| tempInput.pop_back(); | ||
| } | ||
|
|
There was a problem hiding this comment.
неэффективный метод с копированием
| return outputDat.back(); | ||
| } | ||
|
|
||
| const int& Queue::Front() const { |
There was a problem hiding this comment.
методы Front неконсистенты между собой, используют разные подходы
| Queue::Queue(std::initializer_list<int> initList) : inputDat(initList) {} | ||
|
|
||
| Queue::Queue(const std::stack<int>& stack) { | ||
| std::stack<int> temp = stack; |
There was a problem hiding this comment.
нужно тогда сразу принимать копию, иначе не понятно, что это не очень эффективный метод
| std::stack<int> temp = stack; | ||
| // Извлекаем элементы из стека в обратном порядке | ||
| while (!temp.empty()) { | ||
| inputDat.insert(inputDat.begin(), temp.top()); |
There was a problem hiding this comment.
производить вставку в начало вектора очень неэффективно, когда есть контейнер output
| } | ||
|
|
||
| Phasor operator/(const Phasor& phasor, double scalar) { | ||
| Phasor result = phasor; |
There was a problem hiding this comment.
может тогда Phasor принимать как копию?
18thday
left a comment
18thday
left a comment
There was a problem hiding this comment.
@Dsv9toy отметил следующие моменты, требующие исправления:
- использование тела конструктора вместо списка инициализации
- неэффективно сравнение в Queue с копированием
- неэффектривный метод Front с копированием в Queue
- неэффективный конструктор от стека в Queue
3 participants
No description provided.