Мирошниченко Евгений #22
Conversation
|
|
||
|
|
||
| class Stack { | ||
| public: |
There was a problem hiding this comment.
лишнии отступы, для модификаторов как правило не делают отступ и следовательно все методы имеют лишний отступ
There was a problem hiding this comment.
Как я понимаю, зависит от кодстайла
Поэтому не стал менять
04_week/tasks/stack/stack.cpp
Outdated
| data_.clear(); | ||
| } | ||
| void Swap(Stack& other){ | ||
| std::swap(this->data_, other.data_); |
There was a problem hiding this comment.
this-> не используют в данном случае
There was a problem hiding this comment.
Мне лично так просто понятнее. Но как понимаю, не по кодстайлу
0ad4174
04_week/tasks/stack/stack.cpp
Outdated
| return true; | ||
| } | ||
| bool operator!=(const Stack& other) const{ | ||
| return ! (*this==other); |
There was a problem hiding this comment.
пробел после унарного оператора лишний
| counter_ = 0; | ||
| pos_for_next_ = 0; | ||
| oldest_ = 0; | ||
| } |
There was a problem hiding this comment.
правильнее использоватьсписок инициализации вместо тела конструктора
There was a problem hiding this comment.
Есть reserve для data_, поэтому использовал классический конструктор
| *iter = init_val; | ||
| } | ||
| pos_for_next_ = 0; | ||
| counter_ = size > 0 ? size : 1; |
There was a problem hiding this comment.
в данном случае аналогично, у вектора есть конструктор который может создать вектор нужного размера с нужным значением
There was a problem hiding this comment.
Сделал через assign, так вектор уже существует
0ad4174
| if (counter_ == size_) { | ||
| return false; | ||
| } | ||
| Push(val); |
There was a problem hiding this comment.
в данном методе дважды выполняется проверка counter_ == size_ в методе и в Push, можно переиспользовать код без дополнительных првоерок
There was a problem hiding this comment.
3 общих строчки не стал уж выносить в общий метод
| size_ = new_size; | ||
| data_ = new_data; | ||
| counter_ = new_counter; | ||
| } |
There was a problem hiding this comment.
очень много кода получилось
| size_t size_; | ||
| size_t pos_for_next_; | ||
| size_t oldest_; | ||
| size_t counter_; |
There was a problem hiding this comment.
можно было обойтись количеством полей на 1-2 меньше
There was a problem hiding this comment.
Да, наверное oldest_ можно было выразить через pos_for_next_ и counter_, например
04_week/tasks/queue/queue.cpp
Outdated
| in_.reserve(size); | ||
| } | ||
| Queue(std::vector<int> vector){ | ||
| in_ = vector; |
There was a problem hiding this comment.
в списке инийиализации
04_week/tasks/queue/queue.cpp
Outdated
| stack.pop(); | ||
| } | ||
|
|
||
| std::reverse(temp.begin(), temp.end()); |
There was a problem hiding this comment.
почему сразу не складывать в out_? без лишних копирований
There was a problem hiding this comment.
Согласен, так проще
0ad4174
04_week/tasks/queue/queue.cpp
Outdated
| in_.reserve(initList.size()); | ||
| for (int val : initList) { | ||
| in_.push_back(val); | ||
| } |
There was a problem hiding this comment.
у вектора есть конструктор от класса список инициализации
04_week/tasks/queue/queue.cpp
Outdated
| else { | ||
| // Если есть нет, то с конца последнего | ||
| return out_.front(); | ||
| } |
There was a problem hiding this comment.
тернарный оператор, комментарии лишние
04_week/tasks/queue/queue.cpp
Outdated
| in_.clear(); | ||
| out_.clear(); | ||
| in_.shrink_to_fit(); | ||
| out_.shrink_to_fit(); |
There was a problem hiding this comment.
shrink_to_fit() - не принято делать, clear обычно не освобождает память
There was a problem hiding this comment.
Согласен, size и без этого будет 0
0ad4174
04_week/tasks/queue/queue.cpp
Outdated
| void Swap(Queue& other){ | ||
| std::swap(in_, other.in_); | ||
| std::swap(out_, other.out_); | ||
|
|
There was a problem hiding this comment.
лишняя пустая строка
| return false; | ||
| } | ||
| Queue one = *this; | ||
| Queue two = other; |
There was a problem hiding this comment.
это слишком неэффективно, устройство очереди известно, поэтому в данном случае копирование слишком дорого, его можно избежатьсравнив элементы
There was a problem hiding this comment.
Для сравнения надо оба списка привести в один порядок, так как в одном списке часть элементов может быть в out_, а в другом в in_. Так как == должно работать в том числе с константными объектами, просто вызвать для них in_to_out не получается.
Сходу не вижу решения, как без создания нового списка с правильным порядком сравнить 2 таких структуры
04_week/tasks/queue/queue.cpp
Outdated
| } | ||
|
|
||
| bool operator!=(const Queue& other) const{ | ||
| return ! (*this == other); |
04_week/tasks/queue/queue.cpp
Outdated
|
|
||
| protected: | ||
| std:: vector <int> in_{}; | ||
| std:: vector <int> out_{}; |
There was a problem hiding this comment.
это очень плохо, много лишних пробелов
04_week/tasks/queue/queue.cpp
Outdated
| return ! (*this == other); | ||
| } | ||
|
|
||
| protected: |
There was a problem hiding this comment.
зачем модификатор protected?
There was a problem hiding this comment.
Какое-то legacy
0ad4174
| std:: vector <int> in_{}; | ||
| std:: vector <int> out_{}; | ||
|
|
||
| void In_to_out(){ |
There was a problem hiding this comment.
Разный стиль именования методов
There was a problem hiding this comment.
Не понял, о чем речь
Все мои методы snake_case с большой буквы
04_week/tasks/queue/queue.cpp
Outdated
|
|
||
| out_.swap(new_out); | ||
| in_.clear(); | ||
| in_.shrink_to_fit(); |
There was a problem hiding this comment.
зачем его обрезать?
There was a problem hiding this comment.
Согласен, size и без этого будет 0
0ad4174
04_week/tasks/phasor/phasor.cpp
Outdated
| } | ||
|
|
||
| Phasor(double ampl, double angle, ExpTag tag){ | ||
| (void)tag; |
There was a problem hiding this comment.
в С++ лучше [[maybe_unused]] в аргументах перед ExpTag tag, либо static_cast(tag)
There was a problem hiding this comment.
Да, забыл что с-style лучше не использовать
0ad4174
04_week/tasks/phasor/phasor.cpp
Outdated
|
|
||
| Phasor& operator*=(Phasor& lhs, const Phasor& rhs) { | ||
| lhs.SetCartesian(lhs.Real() * rhs.Real() - lhs.Imag() * rhs.Imag(), | ||
| lhs.Real() * rhs.Imag() + lhs.Imag() * rhs.Real()); |
There was a problem hiding this comment.
выравнивание похало
There was a problem hiding this comment.
@EugeniusMiroshnichenko по задачам на классы:
- использование тело конструктора вместо списка инициализации
- использование this->
- неэффективно сравнение в Queue с копированием
- неэффективный конструктор от стека в Queue
- поправить стиль кода
2 participants
No description provided.