WW #12

[Sokolov Viacheslav]

Для include/my_vector_impl.hpp не хватает include guard-а

10 std::ostream &operator<<(std::ostream &, const product::Product &);
зачем помещать в глобальное пространство имен?

38 Product(Product &&other) : name_{nullptr}, quantity_{0}, price_{0} {
39 swap(other);
40 }
41
42 Product &operator=(Product &&other) {
43 swap(other);
44 return *this;
45 }

noexcept

30 assert(vec1.capacity() == ceil_power_of_two(vec1.size()));
можно и size заодно проверить

не все методы покрыты тестами, например, rvalue и [] не тестируется

14 my_vector &operator=(my_vector other);
15 my_vector(my_vector &&other);

noexcept

25 T operator[](std::size_t index) const;

const& noexcept - непонятно, зачем копировать

30 void clear();
noexcept

52 template <typename T> void test_minimum_demands(T a, T b);
53
54 template <typename T> void test_default_constructible();
55
62 #include "my_vector_test.hpp"

непонятно, зачем это в my_vector.h

58 template <typename T>
59 std::ostream &operator<<(std::ostream &, const my_vector::my_vector<T> &);

зачем выносить в глобальное пространство имен?

10 inline std::size_t ceil_power_of_two(std::size_t n) {
11 assert(n < SIZE_MAX / 2);
12 if (n == 0)
13 return 1;
14 std::size_t ans = 1;
15 n--; Для того чтобы не разбирать случай n = 2^{k
16 while (n >= 1)
17 n >>= 1, ans <<= 1;
18 return ans;
19 }}

лучше (с точки зрения организации кода, но не с точки зрения производительности) не inline, а в .cpp

24 try {
25 array_ = static_cast<T *>(aligned_alloc(alignof(T), capacity_ * sizeof(T)));
26 } catch (...) {
27 if (array_)
28 free(array_);
29 throw;
30 }

какое здесь ожидается исключение?

54 std::unique_ptr<T[]> tmp(new T[n]);

не очень хорошо с точки зрения потребления памяти, лучше было бы создавать по одному

162 vector_holder v{0};
163 vholder_.swap(v);
при этом capacity станет 0. У std::vector же capacity не меняется; не очень понятно, в какой ситуации capacity=0 - желаемое поведение.

156 template <typename T> void my_vector<T>::pop_back() {
с такой реализацией - noexcept

120 if (n < vholder_.capacity_)
121 return;
<=

18 template <typename T> void my_vector<T>::reserve(std::size_t n) {
119 assert(n == ceil_power_of_two(n));
зачем это ограничение?

resize реализован неоптимально.
1) если n <= size_, то нужно просто освободить память.
2) если size_ < n < capacity_, то аллокация памяти не нужна
3) даже если аллокация нужна, стоит использовать std::move, а не копирование. Для этого нужно сначала заполнить конец массива, заполнив значениями по умолчанию, а потом сделать move уже имеющихся элементов.

В push_back не нужно ничего копировать. reserve + move дают нужный эффект, единственное возможное исключение - это bad_alloc.

54 std::unique_ptr<T[]> tmp(new T[n]);

для примитивных типов память останется неинициализированной. Нужно использовать

54 std::unique_ptr<T[]> tmp(new T[n]());

​https://stackoverflow.com/a/7546745

[Sokolov Viacheslav]

153 if (vholder_.size_ >= vholder_.capacity_) {
154 reserve(vholder_.capacity_ == 0 ? 1 : 2 * vholder_.capacity_);
155 }
проще безусловно reserve(size() + 1)

130 if (n < vholder_.capacity_)
131 return;
<=

129 void my_vector<T>::reserve(std::size_t n) {
текущая реализация нарушает требование

Методы и конструкторы, которые увеличивают capacity, делают его равным минимальной достаточной степени двойки.

т.е. reserve могут позвать на произвольное значение, а внутри метода должно получиться так, что новое capacity - минимальная необходимая степень двойки

114 vholder_.ctor_elems(n - vholder_.size_);
здесь может произойти исключение, нарушится строгая гарантия

127 for (size_t i = 0; i < n - vholder_.size_; i++, v.size_++) {
128 new (v.array_ + vholder_.size_ + i) T{};
129 }
если здесь произойдет исключение, то будет нарушено соглашение "первые size полей инициализированы", что используется в деструкторе. Стоит обработать через try.. catch.. throw;

118 vector_holder v{2 * vholder_.capacity_};
не факт, что этого достаточно, чтобы вместить n элементов

34 new (array_ + size_++) T(data[i]);

здесь и в аналогичных местах size_ нужно увеличивать строго после выполнения конструктора, потому что если из него вылетит исключение, то size_ будет больше, чем реально сконструировано объектов, что важно в деструкторе