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

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

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

[Филипп]

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

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

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. При умножении эффективнее суммировать в локальную переменную, а затем делать присваивание.

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

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

Не могли бы Вы мне посоветовать, как лучше разобраться с первым? Выбрасывать исключение, если
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. При умножении эффективнее суммировать в локальную переменную, а затем делать присваивание.

[Филипп]

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

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

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

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

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

Спасибо!