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
Type: | ожидается проверка → ожидаются исправления |
---|
comment:2 Changed 4 years ago by
Про тесты понял, это же smoke из условия. Покройте тестами то, что не работает.
comment:3 Changed 4 years ago by
Type: | ожидаются исправления → ожидается проверка |
---|---|
Version: | 1.0 → 2.0 |
comment:4 follow-up: 5 Changed 4 years ago by
/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 Changed 4 years ago by
Поправил
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
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
Type: | ожидаются исправления → ожидается проверка |
---|---|
Version: | 2.0 → 3.0 |
comment:8 Changed 4 years ago by
Resolution: | → задача сдана |
---|---|
Status: | assigned → closed |
Просьба разбить обьявление и определение. Так стоит делать, чтобы отдельно смотреть на интерфейс, отдельно - на реализацию. Зачастую интересна не реализация, а интерфейс.
11 class my_array
final?
assert все же стоит поставить.
48 void fill(T val)
какая мотивация принимать здесь по значению, но не модифицировать полученный объект? стоит либо получать по константной ссылке, либо все же подвинуть val куда-нибудь
final?
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
а rvalue?
101 bool at(std::size_t index) const
102 {
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
Тесты не проверяют, что вернут разные методы, а только лишь проверяют наличие этих методов (причем эту задачу можно решить эффективнее, на этапе только лишь компиляции)