WW #12

[Sokolov Viacheslav]

В тестах не хватает проверок методов.
Например, rvalue- и [] методы не протестированы.

new[0] может бросить исключение, поэтому в текущем виде my_vector_holder(0) - не noexcept, а вслед за ним и
15 my_vector(my_vector && other) НЕ noexcept;
​https://stackoverflow.com/a/1087066

24 template <typename T>
25 my_vector<T>::my_vector() : vector_impl_(my_vector_holder(0)) {}

с такой реализацией стоит пометить как noexcept

27 T& operator[](std::size_t index) const noexcept;

здесь нарушается const-корректность: const метод предоставляет не константный доступ к данным. Стоит разбить на две версии (const и не const)

31 void clear();

noexcept?

32 std::ostream& operator<< (std::ostream& os) const;
лучше именовать >>, чтобы x >> os согласовывлось с os << x

36 size_t found_cap(size_t capacity) const noexcept;
из названия неясно, что этот метод делает

43 void swap(my_vector_holder &);

noexcept

47 size_t size_ = 0;

какая мотивация инициализировать в этом месте?

52 while (deg < capacity) {
53 deg *= 2;
54 }

можно переполниться и зациклиться. Наверное, есть какое-то предположение на capacity.

73 template <typename T>
74 void my_vector<T>::my_vector_holder::swap(my_vector_holder & other) {
75 std::swap(array_, other.array_);
76 std::swap(capacity_, other.capacity_);
77 std::swap(size_, other.size_);
78 }

поближе к другим методам класса my_vector_holder?

81 my_vector<T>& my_vector<T>::operator= (my_vector other) {

noexcept?
(этот метод несовместим с

87 my_vector<T>& my_vector<T>::operator= (my_vector && other) noexcept {

• напишите тест)

99 new (tmp_vec.array_ + tmp_vec.size_) T(vector_impl_.array_[tmp_vec.size_]);

std::move

113 for (; tmp_vec.size_ < vector_impl_.size_; tmp_vec.size_++) {
114 new (tmp_vec.array_ + tmp_vec.size_) T(vector_impl_.array_[tmp_vec.size_]);
115 }
116 for (; tmp_vec.size_ < newsize; tmp_vec.size_++) {
117 new (tmp_vec.array_ + tmp_vec.size_) T();
118 }
лучше поменять порядок операций: сначала позвать конструкторы по-умолчанию, а потом позвать move от тех элементов, которые уже есть.
Кроме того, это все можно сделать без дополнительных аллокаций, если и так достаточно места.

124 T& my_vector<T>::operator[](std::size_t index) const noexcept {
125 return vector_impl_.array_[index];
126 }
не хватает проверки "выход за границы массива"

94 if (newcap < vector_impl_.capacity_) {
95 return;
96 }

<= ?

130 if (vector_impl_.size_ + 1 > vector_impl_.capacity_) {
131 reserve(vector_impl_.capacity_ + 1);
132 }
можно просто
131 reserve(vector_impl_.capacity_ + 1);

133 new (vector_impl_.array_ + vector_impl_.size_) T(t);
std::move(t)

137 void my_vector<T>::pop_back() {
138 vector_impl_.array_[--vector_impl_.size_].~T();
139 }
не хватает проверки контракта "вектор не пустой"

155 for (size_t i = 0; i < vector_impl_.size_; i++) {
156 os << vector_impl_.array_[i] << ' ';
157 }
по условию после последнего элемента не должно быть пробела

[Sokolov Viacheslav]

Не хватает включения <cassert>

constexpr static int MAX_CAPACITY = 1073741824;
что это за магическая константа?

60 if (capacity > MAX_CAPACITY) {
61 return MAX_CAPACITY;
62 }

>= ?
и хорошо бы assert поставить, что <.

130 for (; vector_impl_.size_ < newsize; vector_impl_.size_++) {
131 new (vector_impl_.array_ + vector_impl_.size_) T();
132 }
здесь может произойти исключение, которое в данном случае нужно обработать (и прокинуть дальше)

124 for (; tmp_vec.size_ < vector_impl_.size_; tmp_vec.size_++) {
125 new (tmp_vec.array_ + tmp_vec.size_) T(std::move(vector_impl_.array_[tmp_vec.size_]));
126 }
здесь что-то не то происходит.

121 for (; tmp_vec.size_ < newsize; tmp_vec.size_++) {
122 new (tmp_vec.array_ + tmp_vec.size_) T();
123 }
кроме того, в этом месте уже произошла аллокация в ту область памяти, куда нужно сделать std::move, поэтому через new второй раз неправильно это делать.

[Sokolov Viacheslav]

В include/my_vector_impl.h используется assert, но не подключен cassert

constexpr static int MAX_CAPACITY = 1073741824; // 2**30
можно же просто 1<<30? Тогда и комментарий не нужен. Тип лучше было бы использовать size_t.
По достижении этого размера программа перестанет работать корректно, но ни разработчик, ни пользователь об этом не узнают (из-за текущей реализации next_pow2).

122 for (size_t i = vector_impl_.size_; i < newsize; i++) {
123 new (tmp_vec.array_ + i) T();
124 }
если исключение произойдет в очередном T(), предыдущие созданные объекты не будут разрушены, поскольку tmp_vec.size_ == 0 в этот момент.