Opened 7 years ago

Closed 6 years ago

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

HA1 [Константинов Антон Владимирович]

Reported by: Антон Владимирович Константинов Owned by: rutsky,grabovoy.philipp
Priority: проверка Milestone: ha1-milestone2
Component: HA#1 matrices Version:
Keywords: Cc:

Description

Проверьте, пожалуйста, домашнее задание.

Change History (6)

comment:1 Changed 7 years ago by Филипп

Приветствую!

Есть несколько вещей на доработку:

  1. Конструкторы и Matrix::allocate() могут инициализировать динамические массивы длиной 0, обращатся к ним по индексам (как, например, происходит в деструкторе) -- undefined behavior (пишут на http://www.cplusplus.com/reference/new/operator%20new%5B%5D)
  2. Опасно в публичном operator[] выдавать указатель без проверок. В текущей версии не проверяется индекс строки, и потом никак не проверить номер столбца -- можно вылезки за границы.

И просто полезные замечания:

  1. Стоит использовать список инициализации в конструкторе вместо явного присваивания (rows, cols)
  2. (*this)[i][j] лучше заменить на обращение к приватному полю (row_ptr_array_[i][j]), чтобы избежать вызова метода (хотя, возможно, компилятор это соптимизирует).
  3. При умножении эффективнее суммировать в локальную переменную, а затем делать присваивание.

comment:2 Changed 7 years ago by Филипп

Milestone: ha1-milestone1ha1-milestone2
Type: ожидается проверкаожидаются исправления

comment:3 in reply to:  1 Changed 7 years ago by Антон Владимирович Константинов

Здравствуйте! Спасибо за замечания.

Не могли бы Вы мне посоветовать, как лучше разобраться с первым? Выбрасывать исключение, если
rows или cols -- нулевые?

Replying to grabovoy:

Приветствую!

Есть несколько вещей на доработку:

  1. Конструкторы и Matrix::allocate() могут инициализировать динамические массивы длиной 0, обращатся к ним по индексам (как, например, происходит в деструкторе) -- undefined behavior (пишут на http://www.cplusplus.com/reference/new/operator%20new%5B%5D)
  2. Опасно в публичном operator[] выдавать указатель без проверок. В текущей версии не проверяется индекс строки, и потом никак не проверить номер столбца -- можно вылезки за границы.

И просто полезные замечания:

  1. Стоит использовать список инициализации в конструкторе вместо явного присваивания (rows, cols)
  2. (*this)[i][j] лучше заменить на обращение к приватному полю (row_ptr_array_[i][j]), чтобы избежать вызова метода (хотя, возможно, компилятор это соптимизирует).
  3. При умножении эффективнее суммировать в локальную переменную, а затем делать присваивание.

comment:4 Changed 7 years ago by Филипп

Проще всего проставлять nullptr при отсутствии элементов (rows = 0, cols = 0) и в деструкторе это проверять.

comment:5 Changed 7 years ago by Антон Владимирович Константинов

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

Ещё раз здравствуйте! Проверьте, пожалуйста, исправленную программу.

Пара комментариев:

  1. Проверять на nullptr пришлось бы во всех методах, которые так или иначе обращаются к row_ptr_array_ под индексам, поэтому я явно запретил создание матрицы нулевого размера (это подходит под условия задачи, так как в формулировке чётко указано, что размеры матрицы -- натуральные числа). Сделал я это бросанием исключения при попытке сделать allocate с нулевым размером. Оба конструктора вызывают allocate, поэтому враг не проскочит.
  2. Добавил прокси-класс MatrixRow и проверки на соответствие индексов допустимому диапазону. Теперь, если пытаемся выйти за границу, вылетит исключение.
  3. Списки инициализации, прямое обращение к приватному полю, суммирование в локальную переменную -- исправил.

Спасибо!

comment:6 Changed 6 years ago by Филипп

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