Opened 4 years ago

Closed 4 years ago

#819 closed ожидается проверка (задача сдана)

WW #12

Reported by: Filippov Denis Owned by: Sokolov Viacheslav
Component: WW_vector Version: 3.0
Keywords: Cc:

Description


Change History (6)

comment:1 Changed 4 years ago by Sokolov Viacheslav

30 void clear();

noexcept?

40 ~my_vector_holder() noexcept;

деструктор по-умолчанию noexcept

44 static std::size_t toDegreeOf2(std::size_t num) noexcept;

из названия неясно, что "вверх"
Кроме того, от параметра T не зависит, поэтому есть смысл вынести в свободную функцию, а реализацию отправит в .cpp

include/my_vector.h
возможно, стоит подключить в конце файла include/my_vector_impl.hpp, чтобы сценарий использования шаблонного кода не отличался от сценария использования нешаблонного (подключили заголовочный файл с объявлением и все работает).

15 std::size_t rSize = sizeof(int32_t) * 8, lSize = 0;
почему int32_t?
В таком виде можно просто писать 32.
На num есть какое-то предположение?

17 while (lSize < rSize)
18 {
19 med = (lSize + rSize + 1) / 2;
20 if (std::size_t(1 << med) <= num)
21 lSize = med;
22 else
23 rSize = med - 1;
24 }

Бинпоиск проще писать в виде "для левого конца гарантировано верно, для правого гарантировано неверно" (что (1<<x) < num):

17 while (lSize + 1 < rSize)
18 {
19 med = lSize + (rSize - lSize) / 2;
20 if ((std::size_t{1} << med) < num)
21 lSize = med;
22 else
23 rSize = med;
24 }
25 return 1<<rSize;

кроме того, l,r,m будут понятнее, чем lSize, rSize, med

29 template<typename T>
30 my_vector::my_vector<T>::my_vector_holder::my_vector_holder(std::size_t cap)
31 : _cap(toDegreeOf2(cap)),
32 _len(0),
33 _data(reinterpret_cast<T *>(new char [_cap * sizeof(T)]))
34 {
35 }
в такой реализации конструктор по-умолчанию не noexcept, а это требуется для noexcept move-конструктора

41 _data[_len-- - 1].~T();
--_len?

56 new (_holder._data + _holder._len) T(other._holder._data[_holder._len]);

other[_holder._len]?

116 if (size < _holder._len)
в этом случае аллокация избыточна.

123 for (; newHolder._len < _holder._len; newHolder._len++)
124 new (newHolder._data + newHolder._len) T(std::move(_holder._data[newHolder._len]));
125 for (; newHolder._len < size; newHolder._len++)
126 new (newHolder._data + newHolder._len) T;
здесь нарушается строгая гарантия: конструктор по-умолчанию может бросить исключение, в результате чего исходный объект окажется в сломанном состоянии (его данные-то уже подвинулись).

133 const T & my_vector::my_vector<T>::operator[](std::size_t i) const
134 {
135 return _holder._data[i];
136 }
стоит проверить, что выполняется контракт "считывание из валидной области"

151 template<typename T>
152 void my_vector::my_vector<T>::pop_back()
153 {
154 _holder._data[--_holder._len].~T();
155 }
не хватает проверки контракта "объект не пуст"

168 for (size_t i = 0; i < vec.size() - 1; i++)
169 out << vec[i] << ' ';
170 out << vec[vec.size() - 1];
size() может быть 0

Progduct
typo

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

comment:2 Changed 4 years ago by Sokolov Viacheslav

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

comment:3 Changed 4 years ago by Filippov Denis

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

comment:4 Changed 4 years ago by Sokolov Viacheslav

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

toUpDegreeOf2 лучше upTo

в реализации может быть переполнение при большом входном num, стоит зафиксировать предусловие

при предыдущей проверке пропустил

42 new (_holder._data + _holder._len) T;

120 new (newHolder._data + newHolder._len) T;
здесь объект останется неинициализированным, нужно T{} / T()

150 if (_holder._len)
151 _holder._data[--_holder._len].~T();
в случае _len == 0 вызов pop_back - ошибка программиста. Ее лучше ловить с помощью assert, текущий вариант противоречит fail fast и затрудняет поиск проблемы.

117 for (; newHolder._len < _holder._len; newHolder._len++)
118 new (newHolder._data + newHolder._len) T(_holder._data[newHolder._len]);
119 for (; newHolder._len < size; newHolder._len++)
120 new (newHolder._data + newHolder._len) T;

resize можно реализовать без копирований, используя std::move (по условию просят избегать копирований). При этом в T{} может произойти исключение, поэтому в идеале нужно сначала создать объекты T{}, а потом сделать std::move уже имеющихся.

comment:5 Changed 4 years ago by Filippov Denis

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

comment:6 Changed 4 years ago by Sokolov Viacheslav

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