HA1

[Andrey]

Владимир, добрый вечер.

Я загрузил домашнее задание в понедельник ночью до дедлайна, но упустил информацию, что нужно было создать карточку.
Посмотрите, пожалуйста, мою работу.

Андрей Пестов
andreypestov82@…

[Vladimir Rutsky]

Андрей, здравствуйте!

Замечания:

1. Заголовочный файл в данной задаче должен называться matrices.hpp.

2. Включайте собственные файлы используя #include "matrices.hpp", а не синтаксис включения системных заголовочных файлов (#include <matrices.hpp>).

3. Это некорректный страж включения:

#ifndef MATRICES_H
#define MATRICES_H

#endif // MATRICES_H

#include <stdlib.h>
class matrix
{
...

4. Включайте C++ версию Си заголовочных файлов: #include <cstdlib> вместо #include <stdlib.h> (если вам этот заголовочный файл действительно необходим).

5. В программе есть утечки памяти, например, память результирующей матрицы не освобождается.

6. Вместо int 1, 2 для операций используйте enum.

7. Не вызывайте явно деструктор (A->~matrix();) --- это очень специфичная операция. Либо вынесите освобождение памяти в отдельный метод (clear()), либо удаляйте matrix с помощью delete.

[Vladimir Rutsky]

Используйте один тикет для одной задачи, как указано ​здесь.

[Vladimir Rutsky]

Замечания:

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

Matrix * readMatrix (string const & file_name)

2. Нельзя использовать exit() для нормального выхода из программы. При вызове exit() не раскручивается стек и не вызываются деструкторы, т.е. не происходит никакой (вероятно важной) деинициализации объектов.

3. Нет необходимости явно закрывать файл:

{
    ifstream matrix_file;
    matrix_file.open (file_name);

    ...

    matrix_file.close();
}

он будет закрыт, когда переменная matrix_file будет уничтожена при выходе из области видимости.

Плюс, вы можете открыть файл при конструировании ifstream: ifstream matrix_file(file_name);.

4. Страж включения должен оборачивать весь файл:

#include <fstream>
#ifndef MATRICES_HPP
#define MATRICES_HPP
...

5. Условие в этом методе всегда истинно:

void Matrix::clear()
{
    if (this != nullptr)
        delete [] data_;
}

Возможно вы имели в виду if (data_ != nullptr)?
Кстати, безопасно вызывать delete nullptr и delete [] nullptr --- это определённые операции, которые ничего не делают (т.е. внутри delete есть проверка, является ли аргумент nullptr).

6. clear() и ~Matrix() дублируют друг-друга. Вызывайте из ~Matrix clear().

7. В программе всё ещё есть утечки памяти. Например, вы освобождаете память от результирующей матрицы:

 A->printMatrix();
 A->clear();

но не освобождаете память в A: A был получен как new Matrix, соответственно теперь где-то нужно сделать delete A.

Аналогично со всеми остальными объектами Matrix --- вы выделяете их в куче, но не освобождаете.
Попробуйте вместо clear() удалять объекты Matrix, тогда они в деструктуре будут освобождать память, выделенную для элементов матрицы.

8. Аккуратно обработайте крайние случае, например, когда программа вызывается без аргументов (сейчас она падает). При обработке всех ошибок вся выделенная память должна быть корректно освобождена.

[Andrey]

Спасибо. Исправил.

[Vladimir Rutsky]

Замечания:

1. Вы объявили Matrix::clear(), но не реализовали его (но и не используете).

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

[Vladimir Rutsky]

Milestone ha1-deadline deleted