WW #14

[Артур Гулецкий (huletski)]

Эх, снова make не работает (+ лишняя папка .idea в решении):

{lab_14}[2083]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/usoltsev.nikita/lab_14
Updating '.':
At revision 3234.
{lab_14}[2084]$ make
make: /Applications/CLion.app/Contents/bin/cmake/mac/bin/cmake: Command not found
Makefile:206: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 127

Замечания:

global

• (для учебный целей) измените реализацию Matrix так, чтобы она хранила данные, непосредственно используя указателях. Так базовые гарантии будут не ootb;

main.cpp

• 6: s/array_matrix/registers;
• 8: s_in лучше заменить на, например, command; в общем случае, давайте имена переменным основываясь не на их типе, а на том, что (по смыслу) она должна хранить;
• 12: зачем считывать в tmp пробел?

matrix.hpp

• реализуйте перегрузку операторов (op*=, op+=, op<<, op>>), чтобы код, использующий Matrix, выглядет естественнее;
• в load лучше передавать std::istream;
• 17, 19: const у аргументов;
• 57: кто должен удалять строку, если исключение бросится так:

  char *msg = new char[...];
  // init msg
  throw MatrixException(msg);
  

-> храните сообщение в RAII объекте внутри класса (std::string) либо храните копию сообщения и удаляйте в деструкторе;

matrix.cpp

• 7-10: у std::vector есть конструктор, позволяющий задавать начальное число элементов и значение для каждого элемента -> инициализацию лучше вынести в вызов конструктора в списке инициализации;
• 41: субъективно имя product лучше;
• 57: Matrix::load. У потоков есть возможность включить режим работы, при котором бросается исключение, в случае установки опеределнного error flag'а. Кажется, что использование потока в таком режиме сделало бы код проще (ifы на каждый чиx -> catch-block);
• 53, 54: почему не написать *this = buf;?

---

Баллы: 5.

[Артур Гулецкий (huletski)]

Замечания:

• базовые гарантии сломаны в коде загрузки файла (см. ниже);
• ошибки в реализации умножения, обратите внимание на размер матрицы-результата умножения;
• в папке с решением лишняя папка .idea;
• элементы матрицы все еще хранятся в контейнере (std::vector).

matrix.hpp

• перегруженные op>>, op<< сделайте функциями как принято (первым аргументом должен быть поток, а не (неявнo) this);
• неясно зачем Matrix::print нужен, если op<< перегружен;

main.cpp

• (fyi) 13. имена переменных: s/temp/reg_prefix, если после чтения написать assert(reg_prefix == '$')/обработать ситуацию с неверным форматом, станет еще лучше;

matrix.cpp

• 7. строка длиной 100+ символов, код воспринимать сложнее. Лучше, например, так переформатировать:

  Matrix::Matrix(...)
    : row_(row), col_(col)
    , matrix(...) {}
  

• расстановка фигурных скобок для тела if, состоящего из одного statement'a: где-то ставите (55), где-то нет (36). Выберите один стиль и придерживайтесь его (субъективно лучше ставить);
• 38: а вот и баг с размерами;
• Matrix::op>>. Базовые гарантии нарушены: если в файле одно или два числа (размеры матрицы без элементов), то метод изменит размеры полей матрицы без изменения содержимого -> нарушится инвариант класса (row, col описывают количество строк и столбцов матрицы) -> проверка в elem может работать неверно. Fix: обновляйте состояние объекта после того, как прочли матрицу полностью.
• 71: неясно зачем в буфер читать, а не сразу в элемент матрицы: is >> buffer.matrix[i][j].

---

Баллы: все еще 5 (с т.з. корректности, похоже, стало даже хуже, т.ч. баллы остаются за предыдущую версию). Можете доделывать.

FYI: для решений на базе векторов максимум в 4 группе 7.5 баллов, а не 10.

[Артур Гулецкий (huletski)]

Дедлайн.