Гатин Ленар #24
Гатин Ленар #24
Conversation
…ernary operators to remove code duplication
…nature of function
…e output of symbols
| bool operator==(const Stack& other) const; | ||
| bool operator!=(const Stack& other) const; | ||
| private: | ||
| int m_null_value = 0; |
There was a problem hiding this comment.
это не соответствует условию, в каждом экземпляре класса хранится лишнее поле, не хорошо
| int m_null_value = 0; | ||
| size_t m_begin = 0; | ||
| size_t m_end = 0; | ||
| size_t m_size = 0; |
There was a problem hiding this comment.
лишние поля, можно было обойтись 2, ну иликак минимум без m_null_value
| int m_last() const; | ||
| }; | ||
|
|
||
| size_t RingBuffer::m_check_zero_size(const size_t size) const { |
There was a problem hiding this comment.
методы должны иметь имя в стиле методов, а не полей
| } | ||
|
|
||
| RingBuffer::RingBuffer(size_t size) { | ||
| m_buffer.reserve(m_check_zero_size(size)); |
There was a problem hiding this comment.
это должно бытыть в списке инициализации
| m_buffer.reserve(m_check_zero_size(size)); | ||
| for (size_t i = 0; i < Capacity(); ++i) { | ||
| m_buffer.push_back(val); | ||
| } |
There was a problem hiding this comment.
это лишний цикл, у вектора есть соответствующий конструктор, и корректней использовать список инициалзиации
| m_buffer.reserve(1); | ||
| } | ||
| m_size = m_buffer.size(); | ||
| m_end = m_buffer.size(); |
There was a problem hiding this comment.
аналогично, список инициализации
| m_copy_core(other); | ||
| } | ||
|
|
||
| inline void RingBuffer::m_push_back_core(const int item) { |
There was a problem hiding this comment.
inline лишняя пометка
| if (Full()) { | ||
| return false; | ||
| } | ||
| else { |
There was a problem hiding this comment.
лишняя вложенность, else не нужно писать, выше в условие есть return
| pop_value = m_buffer[m_begin]; | ||
|
|
||
| m_pop_core(); | ||
|
|
There was a problem hiding this comment.
зачем столько пустых строк
18thday
left a comment
18thday
left a comment
There was a problem hiding this comment.
@GanjaMember по задачам на классы:
- не используются список инициализации в полной мере
- лишнии ручные копирования вместо конструкторов
- лишняя вложенность с else
- много кода при формировании вектора в RingBuffer
- метод Resize в RingBuffer с лишними копированиями
- не оптимальное сравнение с безусловным копированием двух Queue
|
|
||
| void RingBuffer::Pop() { | ||
| if (Size() == 0) | ||
| return; |
There was a problem hiding this comment.
ранее для if в подобных случаях использовался стиль с {}
в данном случае не стиль
| for (size_t i = 0; i < Size(); ++i) { | ||
| vector_buffer.push_back(m_buffer[i]); | ||
| } | ||
| } |
There was a problem hiding this comment.
слишком громоздко для прохождения по буферу, как будто должна быть операция получения следующей позиции и запущен один цикл на количество элементов. не оптимально
| std::vector<int> new_buffer; | ||
| new_buffer.reserve(new_size); | ||
|
|
||
| std::vector<int> vector_buffer = Vector(); |
There was a problem hiding this comment.
второе копирование зачем? не оптимально, несколько раз перекладываем элементы в разные контейнеры
| bool operator==(const Queue& other) const; | ||
| bool operator!=(const Queue& other) const; | ||
| private: | ||
| int m_null_value = 0; |
There was a problem hiding this comment.
лишняя память в каждом экземпляре
| while (!vector.empty()) { | ||
| m_output.push_back(vector.back()); | ||
| vector.pop_back(); | ||
| } |
There was a problem hiding this comment.
а зачем мы принимаем по копии и потом ещё раз перекладываем элементы и к тому же из vector который ничей делаем pop_back
| } | ||
| } | ||
|
|
||
| Queue::Queue(std::vector<int> vector) { |
There was a problem hiding this comment.
нужно использовать список инициализации и сразу складывать в один из двух векторов
| m_output.push_back(m_input.back()); | ||
| m_input.pop_back(); | ||
| } | ||
| } |
There was a problem hiding this comment.
эту операцию можно вынести в приватный метод
| Queue::Queue(std::initializer_list<int> ilist) { | ||
| for (const int v : ilist) { | ||
| m_input.push_back(v); | ||
| } |
There was a problem hiding this comment.
у вектора есть конструктор от initializer_list и им можно воспользоваться в списке инициализации
|
|
||
| bool Queue::operator==(const Queue& other) const { | ||
| std::vector<int> this_merged = m_merge_vectors(m_input, m_output); | ||
| std::vector<int> other_merged = m_merge_vectors(other.m_input, other.m_output); |
There was a problem hiding this comment.
устройство контейнера известно, можно обойтись без копирования, не оптимально создавать две копии больших очередей чтобы сравнить, тем болееесли у них могут быть не равны размеры и можно сразу скаать результат для таких случаев
2 participants
No description provided.