Opened 4 years ago

Closed 4 years ago

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

WW_array

Reported by: tarasov.denis Owned by: Sokolov Viacheslav
Component: WW_array Version: 3.0
Keywords: Cc:

Description


Change History (8)

comment:1 Changed 4 years ago by Sokolov Viacheslav

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

Просьба разбить обьявление и определение. Так стоит делать, чтобы отдельно смотреть на интерфейс, отдельно - на реализацию. Зачастую интересна не реализация, а интерфейс.

11 class my_array
final?

28 const T & operator[](std::size_t index) const
29 {
30 return arr_[index];
31 }
32
33 T & operator[](std::size_t index)
34 {
35 return arr_[index];
36 }

assert все же стоит поставить.

48 void fill(T val)
какая мотивация принимать здесь по значению, но не модифицировать полученный объект? стоит либо получать по константной ссылке, либо все же подвинуть val куда-нибудь

58 class my_array<bool, N>

final?

61 class Proxy_

final?

89 char *byte;
лучше использовать unsigned тип, поскольку signed/unsigned сдвиги работают по-разному (перенос знакового бита)

71 *byte &= ~(1 << bitNumber);
72 *byte |= value << bitNumber;
86 return bool(*byte & (1 << bitNumber));
лучше через static_cast<>, чтобы гарантировать правильный (беззнаковый) сдвиг
bool() - это c-cast, лучше явный static_cast

78 Proxy_ & operator=(Proxy_ &other)

а rvalue?

77 Is needed, otherwise test "A[0] = A[N - 1] = A[0]" fails
здесь-то rvalue версии будут возникать

101 bool at(std::size_t index) const
102 {

103 if (index < 0
index >= N)

104 throw std::out_of_range("Index is out of range");
105 return arr_[index / 8] | (1 << index % 8);
106 }
107
108 bool operator[](std::size_t index) const
109 {
110 return arr_[index / 8] | (1 << index % 8);;
111 }
из-за дублирования кода закралась ошибка

134 char arr_[N / 8 + (N % 8 != 0)];
все верно, по (N % 8 == 0 ? 0 : 1) понятнее, чем неявный каст bool к size_t как 0/1

128 void fill(bool val)
129 {
130 for (size_t i = 0; i < N; ++i)
131 (*this)[i] = val;
132 }
можно эффективнее с помощью memset

Тесты не проверяют, что вернут разные методы, а только лишь проверяют наличие этих методов (причем эту задачу можно решить эффективнее, на этапе только лишь компиляции)

comment:2 Changed 4 years ago by Sokolov Viacheslav

Про тесты понял, это же smoke из условия. Покройте тестами то, что не работает.

comment:3 Changed 4 years ago by tarasov.denis

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

comment:4 Changed 4 years ago by Sokolov Viacheslav

/home/nicesap/HSE/svn/tarasov.denis/lab_13/include/my_array_impl.h:127:11: error: ‘assert’ was not declared in this scope

127 | assert(index < N);

comment:5 in reply to:  4 Changed 4 years ago by tarasov.denis

Поправил

Replying to Sokolov Viacheslav:

/home/nicesap/HSE/svn/tarasov.denis/lab_13/include/my_array_impl.h:127:11: error: ‘assert’ was not declared in this scope

127 | assert(index < N);

comment:6 Changed 4 years ago by Sokolov Viacheslav

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

154 memset(arr_, val * 255, sizeof(arr_));
val * 255 конечно сделает все правильно, но зачем, если
val ? 255 : 0 - не длиннее и понятнее

throw std::out_of_range("Index is out of range");
хорошо бы больше подробностей - по какому индексу было обращение, например

40 operator bool();
const noexcept?

130 return arr_[index / 8] | (static_cast<unsigned char>(1) << index % 8);
ошибка

на самом деле можно все сделать constexpr

82 template<std::size_t N>
83 auto my_array<bool, N>::Proxy_::operator=(const Proxy_ &other) -> Proxy_ &
84 {
85 byte = other.byte;
86 bitNumber = other.bitNumber;
87
88 return *this;
89 }

непонятно, зачем нужна такая реализация

91 template<std::size_t N>
92 auto my_array<bool, N>::Proxy_::operator=(Proxy_ &&other) -> Proxy_ &
93 {
94 *this = static_cast<bool>(other);
95 byte = other.byte;
96 bitNumber = other.bitNumber;
97 other.byte = nullptr;
98 other.bitNumber = 0;
99

100 return *this;
101 }
кажется, с такой реализацией неправильно будет работать
a[i] = a[i + 1] = a[i + 2]

Как видите, тесты плохие. Покройте все укзанные проблемы тестами.

comment:7 Changed 4 years ago by tarasov.denis

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

comment:8 Changed 4 years ago by Sokolov Viacheslav

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