WW #12

[Sokolov Viacheslav]

Не все методы протестированы. Например, move вектора целиком не тестируется.

include/VectorImpl.hpp
не хватает включений заголовочных файлов. Например, assert используется, а <cassert> не подключен.

16 Vector &operator=(Vector other) &;
noexcept

24 void push_back(const T &t) &;
лучше принимать T, чтобы при использованиях
104 v.push_back(std::move(t));
не происходило лишнего копирования

27 std::ostream &operator<<(std::ostream &os) const;

логичнее >>, чтобы v >> os было согласовано с os << v

36 size_t raiseToPowerOf2(size_t x);
static noexcept
Вообще можно вынести наружу и сделать inline / вынести в .cpp

15 data(static_cast<T*>(static_cast<void*>(new char[capacity * sizeof(T)]))) {}
new[0] может бросить исключение, поэтому в текущем виде VectorHolder?(0) - не noexcept, а вслед за ним и
Vector(Vector &&other) НЕ noexcept;
​​https://stackoverflow.com/a/1087066

31 for (; (1U << i) < x; i++) {}
32 return (1 << i);

1) в таком виде может зациклиться - наверное, есть какое-то предположение на x
2) почему в цикле 1U, а в возвращаемом значении 1? Проблема-то та же.

35 template<typename T>
36 const T &Vector<T>::VectorHolder::operator[](size_t index) const noexcept {
37 return data[index];
38 }
39
40 template<typename T>
41 T &Vector<T>::VectorHolder::operator[](size_t index) noexcept {
42 return data[index];
43 }

assert, что нет выхода за границы

100 template<typename T>
101 void Vector<T>::resize(size_t n) & {
102 reserve(n);
103 decreaseSize(n);
104 for (; holder.size < n; holder.size++) {
105 new (holder.data + holder.size) T();
106 }
107 }
T() может бросить исключение, и здесь будет нарушена строгая гарантия. Нужно быть аккуратнее.

114 new (newHolder.data + newHolder.size) T(holder[newHolder.size]);
std::move

140 new (holder.data + holder.size++) T(t);
при копировании может произойти исключение, поэтому скопировать нужно сначала.

[Sokolov Viacheslav]

115 строчкой выше не вызывается конструктор от n,
116 чтобы T() был вызван именно для новых объектов, а не n раз.
117 Кажется, что вызов конструкторов копирования -
118 намного более ожидаемое поведение от resize
119 newVector.reserve(n);
120 for (size_t i = 0; i < n; i++) {
121 newVector.push_back(i < holder.size ? holder[i] : T());
122 }

resize должен делать минимальный набор операций, а именно (в этой ветке):
1) создать n - size новых объектов
2) подвинуть size старых объектов
копирования не нужны, это потенциально дорогая операция, которой стоит избегать.
Также дорогая операция - аллокация памяти, поэтому идеальная реализация включает еще и разбор ситуации, хватает ли текущей capacity, чтобы вместить новые объекты, или нужна дополнительная аллокация. В первом случае обработка через try..catch, во втором - через аллокацию и swap. С точки зрения реализации это все не очень удобно, но это библиотечный код, который для того и нужен, чтобы быть максимально эффективным в заданных предположениях.
Реализовывать предлагается только пункты 1 и 2, то есть избавиться от копирований (это просят в задании). Обращаю внимание, что действия должны быть именно в таком порядке, потому что на стадии 1 могут быть исключения, а на стадии 2 - нет.

132 new (newHolder.data + newHolder.size) T(holder[newHolder.size]);
обязательно std::move. В reserve может произойти исключение только при аллокации, если она удалась, не нужно копировать. move произойдет без исключений, это дешевая операция, в отличие от копирования.

Внести исправления можно до конца модуля.

[Sokolov Viacheslav]

`
113 else if (holder.size < n) {
114 Vector newVector;
115 newVector.reserve(n);
116 for (size_t i = holder.size; i < n; i++) {
117 new (newVector.holder.data + i) T();
118 }
119 for (size_t i = 0; i < holder.size; i++) {
120 new (newVector.holder.data + i) T(std::move(holder[i]));
121 }
122 newVector.holder.size = n;
123 swap(newVector);
124 }
`

Такая наивная реализация не подходит, потому что когда в T() произойдет исключение, будет нарушено предположение Holder-а о том, что в нем первые size (т.е. в данном случае n) элементов инициализированы. Позовется деструктор Holder-а, а в нем будет производиться очистка (~T()) неинициализированной области (первых holder.size элементов).