Opened 6 years ago

Closed 6 years ago

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

Проверка ДЗ №1

Reported by: tankov.vladislav Owned by: rutsky,grabovoy.philipp
Priority: проверка Milestone: ha1-deadline
Component: HA#1 matrices Version: 1.0
Keywords: Cc:

Description


Change History (6)

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

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

Привет!

Несколько вещей для исправлений:

  1. Версию CMake нужно выставлять минимальную необходимую -- это 3.1
  2. Из списка частых ошибок: 6 (название бинаря), 7 (заголовочный для std::invalid_exception)

И пункты-саджесты:

  1. В operator*= из-за вызова конструктора копирования (для leftOp) происходит перевыделение памяти и ее заполнение, хотя на самом деле достаточно "перекинуть" значения вызовом Matrix::Matrix(int, int, double **). Похожей семантикой обладает move-конструктор, о котором будет во второй части курса.
  2. Суммировать при умножении лучше в локальную переменную, а потом раписывать в результирующую ячейку -- чтобы не делать лишних разыменований указателей.
  3. В getNext можно возвращать сразу argv[index], приведение типов выполнится автоматически.
  4. В конструкторах препочительно использовать списки инициализации полей.
Last edited 6 years ago by Филипп (previous) (diff)

comment:2 Changed 6 years ago by tankov.vladislav

Не совсем понял про пункт 7. Я не подключал заголовочный файл для std::invalid_argument (видимо, имелся в виду он) так как он транзитивно получен из других подключенных заголовочный файлов в matrices.cpp. Подключить его, несмотря на то, что он будет избыточен? Пункт 7 вроде говорит о том, что не нужно подключать избыточные заголовочные файлы в своих заголовочных файлах. Но в matrices.hpp у меня только <string>

Last edited 6 years ago by tankov.vladislav (previous) (diff)

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

Да, верно, этого пункта нет в списке частых ошибок (но есть такая частая ошибка): про самодостаточность заголовочных файлов и cpp-шников. <stdexcept> в данном случае нужно включить в matrices.cpp и main.cpp потому что нужное исключение определено в этом заголовочном файле, а его включенность в другие хедеры не гарантируется. Как частный случай: под g++ 4.8.4 (которым я собирал) решение не компилировалось.

И еще один пункт из ошибок: 26. И size_t тоже нужно явно подключать.

comment:4 Changed 6 years ago by tankov.vladislav

Добавил stdexcept и cstddef header. Переделал на size_t индексацию.

comment:5 Changed 6 years ago by tankov.vladislav

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

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

Resolution: задача сдана
Status: newclosed

Нужно только поменять название бинаря: ha1 -> matrices

Note: See TracTickets for help on using tickets.