Opened 4 years ago

Closed 4 years ago

#844 closed ожидаются исправления (задача сдана)

WW #12

Reported by: Bagryanova Ekaterina Owned by: Sokolov Viacheslav
Component: WW_vector Version: 1.0
Keywords: Cc:

Description


Change History (3)

comment:1 Changed 4 years ago by Sokolov Viacheslav

Если я правильно понимаю, то задача была сдана перед deadline-ом, соответственно версия 2 - финальная. Поправьте, если ошибаюсь.

Удобно в использовании не разделять заголовчные файлы с шаблонным кодом и с нешаблонным кодом. Для этого стоит в конце "my_vector.hpp" подключить реализацию (my_vector_impl.hpp)

get_good_capacity - неудачное название, непонятно, что именно метод делает

15 my_vector<T>::my_vector(size_t n) : my_vector() {
16 my_vector<T> result;
17 for (size_t i = 0; i < n; ++i) {
18 result.push_back(T());
19 }
20 *this = std::move(result);
21 }

непонятно, зачем внутри конструктора конструировать объект того же типа. Видимо, чтобы когда вылетит исключение, корректно его обработать и поудалять. На лекциях для решения этой проблемы предлагалось завести вложенный объект, который отвечает только за храенение и разрушение, а пользовательский объект vector уже отвечает за корректность обработки исключений.
Объект считается сконструированным, когда хотя бы один конструктор завершил свою работу. Поэтому в случае использования делегирующего конструктора, когда этот конструктор закончил свою работу и началось выполнение тела my_vector<T>::my_vector(size_t n), если произойдет исключение, позовется деструктор ~my_vector. То есть промежуточный result просто не нужен.
Для эффективности лучше в начале метода позвать reserve(n).

24 my_vector<T>::my_vector(const my_vector<T> &other) : my_vector() {

та же история.

33 my_vector<T>::my_vector(my_vector<T> &&other) noexcept : capacity_(other.capacity_), size_(other.size_), array_(other.array_) {

проще через default ctor + swap

191 for (size_t i = 0; i < vect.size(); ++i) {
192 out << vect[i] << " ";
193 }
последний пробел просят не выводить

172 char *cur = new char[result.capacity_ * sizeof(T)];
стоит проверить, что произведение не переполняется
какая мотивация аллоцировать не T*? (new T[result.capacity_])

171 result.size_ = size_;
но ведь это не соответствует действительности, потому что в сконструированном векторе хранится 0 элементов. Может, убрать параметр size?

141 if (size_ == capacity_) {
142 reserve(capacity_ + 1);
143 }
проще безусловно reserve(size_ + 1)

99 for (size_t i = size_; i < n; ++i) {

100 result.push_back(T());
101 }
здесь может произойти исключение (в конструкторе T()), тогда нарушится strong exception safety, потому что элементы array[i] уже подвинуты. Нужно поменять порядок операций, перемещение элементов производить в конце.

102 *this = result;
std::move(result)

33 Product::Product(const Product &other) : quantity_(other.quantity_), price_(other.price_){
34 name_ = new char[strlen(other.name_) + 1];
35 strcpy(name_, other.name_);
36 }

проще через делегирующий конструктор

120 if (index > size_) {

=

144 new (&array_[size_++]) T(std::move(t));
здесь разыменовывается область памяти, в которой ничего не записано. Это запрещено по Стандарту.

95 for (size_t i = 0; i < size_; ++i) {
96 result.array_[i] = std::move(array_[i]);
97 }

здесь тоже обращение к несконструированным объектам

comment:2 Changed 4 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

comment:3 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed
Note: See TracTickets for help on using tickets.