WW_vector

[Sokolov Viacheslav]

78 try
79 {
80 v.pop_back();
81 assert(true);
82 }
83 catch(...) {}

assert (false)?

assert (true) - все равно, что ничего не писать

Тесты не покрывают rvalue-методы

16 my_vector(my_vector &&other);

нет причин не делать noexcept. noexcept - важная гарантия, которая особенно актуальна при передаче ресурсов.

18 my_vector & operator=(my_vector &&other);

noexcept

17 my_vector & operator=(my_vector other);
18 my_vector & operator=(my_vector &&other);

эти две сигнатуры не могут сосуществовать (добавьте тест и убедитесь в этом)

29 T operator[](std::size_t index) const;
лучше const T&. Непонятно, зачем делать копию; класс может быть некопируемым. Добавьте тест на этот метод (сейчас он не вызывается).

31 void push_back(const T &t);

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

41 vector_holder(vector_holder &&other);

noexcept

44 auto & operator=(vector_holder other);

нет причин использовать auto. Старайтесь избегать auto в декларациях интерфейсов.

53 size_t getMinPowerOfTwo(size_t n);

static; можно вынести наружу (реализацию поместить в .cpp)

16 array_ = static_cast<T *>(std::aligned_alloc(alignof(T), sizeof(T) * capacity_));
какая мотивация использовать aligned_alloc?

40 template<typename T>
41 my_vector<T>::vector_holder::vector_holder(vector_holder &&other) :
42 capacity_{other.capacity_}, size_{other.size_}, array_{other.array_}
43 {
44 other.size_ = 0;
45 other.capacity_ = 0;
46 other.array_ = nullptr;
47 }

можно оформить как : vector_holder(0){swap(*this, other);}

60 std::swap(size_, other.size_);
61 std::swap(capacity_, other.capacity_);
62 std::swap(array_, other.array_);

swap стоит вынести в отдельную функцию

67 template<typename T>
68 size_t my_vector<T>::getMinPowerOfTwo(size_t n)
69 {
70 if (pow(2, (size_t)log2(n)) == n)
71 return n;
72 else
73 return pow(2, (size_t)log2(n) + 1);
74 }

​https://en.cppreference.com/w/cpp/numeric/math/pow
pow - floating point операция. Нет необходимости делать floating point операции для реализации этой функции. Это медленнее и менее прозрачно (может ли вернуться не степень двойки?).
Кроме того, у функции есть неявные предположентия: n не слишком маленькое и не слишком большое. Проверьте контракт.
Название getMinPowerOfTwo не самое говорящее, можно придумать лучше (отразить в названии факт округления вверх - "ceil")

77 void my_vector<T>::vector_holder::copy_elements(const vector_holder &other, size_t count)

может ли совпадать this и &other?
в функции можно было бы звать operator=, а не dtor + ctor.

148 void my_vector<T>::resize(std::size_t n)
149 {
150 if (n < 0)
151 throw std::logic_error("Bad size");
size_t - беззнаковый

159 if (n < holder_.size_)
160 tmp.copy_elements(holder_, n);
в случае уменьшения размера не нужно ничего копировать, нужно просто уменьшить

162 {
163 tmp.copy_elements(holder_, holder_.size_);
164 for (; tmp.size_ < n; ++tmp.size_)
165 tmp.array_[tmp.size_] = T();
166 }
эта реализация также неээфективна: возможно, capacity позволяет сделать resize без дополнительной аллокации. Будет много копирований на пустом месте.

168 holder_ = tmp;
если даже копирования понадобились, временный объект должен отдать ресурсы с помощью std::move

181 holder_ = tmp;
std::move

188 if (index < 0

index >= holder_.size_)

189 throw std::logic_error("Bad index");
< 0?

196 if (index < 0

index >= holder_.size_)

197 throw std::logic_error("Bad index");
< 0?

Можно было бы бросить std::out_of_range

207 tmp.copy_elements(holder_, holder_.size_);
208 new(tmp.array_ + tmp.size_) T(t);
можно сделать эффективнее, используя move.

Формат operator<< для my_vector: <vec[0]> vec[1]> ... <vec[size-1]> (все элементы my_vector через operator<<, разделены ровно одним пробелом, завершающего перевода строки нет).

[tarasov.denis]

Нам говорили, что можно использовать aligned_alloc (на лекции его и использовали) + там всякие бонусы с выравниваниями.
К auto я добавил стрелочку, надеюсь это решает проблему

[Sokolov Viacheslav]

Конечно можно использовать. Мой вопрос в другом - какая мотивация это делать, т.е. зачем? Почему не new[], malloc, calloc, ...?

[tarasov.denis]

Replying to Sokolov Viacheslav:

Конечно можно использовать. Мой вопрос в другом - какая мотивация это делать, т.е. зачем? Почему не new[], malloc, calloc, ...?

Так было в примерах, я решил повторить.

[Sokolov Viacheslav]

110 my_vector<T>::my_vector() : holder_{1}
если вместо 1 сделать 0, то можно пометить как noexcept

69 throw e;

просто throw;

103 for (; size_ > count; --size_)
104 array_[size_ - 1].~T();
нужно позвать деструкторы всех объектов, либо далее использовать не in-place ctor, а operator=

212 void my_vector<T>::reserve(std::size_t n)
n может быть равно 0?

256 new(holder_.array_ + holder_.size_) T(t);
std::move(t).
Всю реализацию можно упростить до
reserve(size_ + 1);
new(array_ + size_) T(std::move(t));
size_++;

189 tmp.array_[tmp.size_] = T();
это не placement new

формат вывода все еще нарушен. Лишний пробел в конце.

​https://stackoverflow.com/a/17657883
из-за этого текущая реализация

57 my_vector<T>::vector_holder::vector_holder(const vector_holder &other) : vector_holder{other.capacity_}

некорректна

[Sokolov Viacheslav]

196 throw e;
просто throw.

49 array_ = static_cast<T *>(std::aligned_alloc(alignof(T), sizeof(T) * capacity_));
в идеале стоило бы проверить, что sizeof(T) * capacity_ не переполняется. То есть что не просят аллоцировать слишком много.

106 my_vector<T>::my_vector(std::size_t n) : holder_{getMinPowerOfTwoCeil(n)}
некорректно отработает для n = 0

212 tmp.copy_elements(holder_, holder_.size_);
такая реализация reserve противоречит требованию

Не создают копии элементов, когда можно обойтись перемещением (без нарушения гарантий).