HA-1: Matrices

[Viacheslav Kukushkin]

Проверьте, пожалуйста.
​http://trac.compscicenter.ru/svn/cpp16/kukushkin.vyacheslav/
commit# 41

[Vladimir Rutsky]

Замечания:

1. Ловите исключения по константной ссылке, если не собираетесь их модифицировать:

 } catch (runtime_error const & e){

2. В C++ принято оформлять в *.cpp/*.c++/*.CC файлах *единицы трансляции*, а в *.hpp/*.h заголовочные файлы, позволяющие переиспользовать код (и допускающие включение в произвольное количество единиц трансляции). Определение в нескольких единиц трансляции одной сущности приведёт к нарушению ​One Definition Rule (ODR).

В данной задаче должно быть две единицы трансляции: main.cpp и matrices.cpp. В matrices.hpp необходимо поместить объявления объектов, определённых в matrices.cpp.

Одинаковые inline функции могут находиться в нескольких единицах трансляции --- это специальное исключение ODR. Методы класса определённые внутри определения класса считаются inline, поэтому определение класса с реализованными методами можно включать в несколько файлов --- переместите определение Matrix в matrices.hpp (кстати, обратите внимание на требуемый суффикс файла).

Чтобы matrices.cpp не пустовал, а также, чтобы удостовериться, что вы понимаете как выносить реализацию методов из определения класса, вынесите реализацию некоторых методов из заголовочного файла matrices.hpp в matrices.cpp (например, Matrix::add и Matrix::mul).

3. Не смешивайте системные и свои инклуды:

#include <iostream>
#include "matrices.h"
#include <string>
# include <fstream>

придерживайтесь единого стиля: либо сначала системные, а затем свои, либо наоборот.
Плюс тут ещё лишний пробел после #.

4. Передавайте объекты, которые не планируете изменять, по константной ссылке:

void add(Matrix const & B) {
Matrix(string const & filepath) {

5. В программе есть утечки памяти. Например, память, выделенная для матрицы A в main() не освобождается.

6. При работе программы наблюдается некорректный доступ к памяти. Например, при запуске ./matrices 5.txt --add 2.txt здесь:

         if (action == "--add") {
             A->add(*B);
         } ...

         delete B;

т.к. add() принимает копию объекта Matrix, а у Matrix не определён корректный оператор копирования, то при выходе из add() локальная переменная B будет уничтожена --- будет вызван деструктор, который освободит память матрицы B из main(), что приведёт к тому, что в delete B в main() будет попытка освободить одну и ту же память второй раз.

[Viacheslav Kukushkin]

Добрый день! исправил ошибки, указанные вами - проверьте, пожалуйста, второй раз.

исправления ошибок по milestone1:

1. ловить исключения по ссылке
2. вынести class Matrix в matrices.hpp
3. отделить системные инклуды от своих
4. const на всё
5. управление памятью: пусть всем занимается RAII

[Matrix *A = new Matrix(...) ==> Matrix A(...)]

[Vladimir Rutsky]

Замечания:

1. Сейчас вы включаете matrices.cpp внутрь main.cpp --- так делать нельзя. Разберитесь, пожалуйста, с единицами трансляции и заголовочными файлами. Если что-то не получается или вы чего-то не понимаете --- смело задавайте вопросы.

Возможно у вас просто не получилось скомпилировать две единицы трансляции?
Вы используете CMake? Попробуйте в CMakeLists.txt в качестве SOURCE_FILES указать и main.cpp и matrices.cpp:

set(SOURCE_FILES main.cpp matrices.cpp)
add_executable(ha1 ${SOURCE_FILES})

2. std::__cxx11::string --- это класс из внутренней области видимости определённой реализации стандартной библиотеки (__cxx11 начинается с двух подчеркиваний), используйте std::string.

3. В matrices.cpp вы делаете using namespace std;, но по факту используете имена с явным указанием области видимости std: std::runtime_error, std::size_t. Можно удалить using namespace std;.

[Viacheslav Kukushkin]

Добрый вечер,
Исправил.
1) Действительно, я включал cpp вместо hpp , т.к. не мог скомпилировать иначе. Настроил Cmake, как вы предложили - всё стало хорошо, спасибо
2) исправил
3) оставил using namespace std, но убрал прямые обращения к типам: std::size_t => size_t

[Vladimir Rutsky]

Решение зачтено.

[Vladimir Rutsky]

Milestone ha1-deadline deleted